* [PATCH v4 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name()
From: Miquel Raynal (Schneider Electric) @ 2026-07-17 15:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, linux-clk,
devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric)
In-Reply-To: <20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com>
Make sure this helper is never broken, especially since we will soon
make some changes in it.
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
---
drivers/clk/clk_test.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index f47f81b7d72a..4084559e157d 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3637,9 +3637,23 @@ static void clk_parse_clkspec_with_incorrect_index_and_name(struct kunit *test)
KUNIT_EXPECT_TRUE(test, IS_ERR(hw));
}
+/*
+ * Verify that of_clk_get_parent_name() returns the correct clock name when
+ * looking up by index through the consumer's clocks property.
+ */
+static void of_clk_get_parent_name_gets_parent_name(struct kunit *test)
+{
+ struct clk_parse_clkspec_ctx *ctx = test->priv;
+ const char *expected_name = "clk_parse_clkspec_1";
+
+ KUNIT_EXPECT_STREQ(test, expected_name,
+ of_clk_get_parent_name(ctx->cons_np, 0));
+}
+
static struct kunit_case clk_parse_clkspec_test_cases[] = {
KUNIT_CASE(clk_parse_clkspec_with_correct_index_and_name),
KUNIT_CASE(clk_parse_clkspec_with_incorrect_index_and_name),
+ KUNIT_CASE(of_clk_get_parent_name_gets_parent_name),
{}
};
--
2.54.0
^ permalink raw reply related
* [PATCH v4 4/7] clk: Improve a couple of comments
From: Miquel Raynal (Schneider Electric) @ 2026-07-17 15:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, linux-clk,
devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric)
In-Reply-To: <20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com>
Avoid mentioning the function names directly in the comments, it makes
them easily out of sync with the rest of the code. Use a more generic
wording.
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
---
drivers/clk/clk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 08874cf9b561..7d63d81ebc09 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -5202,7 +5202,7 @@ static int of_parse_clkspec(const struct device_node *np, int index,
/*
* For named clocks, first look up the name in the
* "clock-names" property. If it cannot be found, then index
- * will be an error code and of_parse_phandle_with_args() will
+ * will be an error code and the OF phandle parser will
* return -EINVAL.
*/
if (name)
@@ -5275,7 +5275,7 @@ of_clk_get_hw_from_clkspec(struct of_phandle_args *clkspec)
*
* This function looks up a struct clk from the registered list of clock
* providers, an input is a clock specifier data structure as returned
- * from the of_parse_phandle_with_args() function call.
+ * from the OF phandle parser.
*/
struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
{
--
2.54.0
^ permalink raw reply related
* [PATCH v4 5/7] clk: Use the generic OF phandle parsing in only one place
From: Miquel Raynal (Schneider Electric) @ 2026-07-17 15:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, linux-clk,
devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric)
In-Reply-To: <20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com>
There should be one single entry in the OF world, so that the way we
parse the DT is always the same. make sure this is the case by avoid
calling of_parse_phandle_with_args() from of_clk_get_parent_name(). This
is even more relevant as we currently fail to parse clock-ranges. As a
result, it seems to be safer to directly call of_parse_clkspec() there,
but doing so implies that we do not try the "clock-ranges" path if we
already found a "clocks" property..
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Fixes: 4472287a3b2f5 ("clk: Introduce of_clk_get_hw_from_clkspec()")
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
---
drivers/clk/clk.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 7d63d81ebc09..45f5d7a4ccc1 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -5219,6 +5219,8 @@ static int of_parse_clkspec(const struct device_node *np, int index,
* has a "clock-ranges" property, then we can try one of its
* clocks.
*/
+ if (of_property_present(np, "clocks"))
+ break;
np = np->parent;
if (np && !of_property_present(np, "clock-ranges"))
break;
@@ -5364,8 +5366,7 @@ const char *of_clk_get_parent_name(const struct device_node *np, int index)
int count;
struct clk *clk;
- rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
- &clkspec);
+ rc = of_parse_clkspec(np, index, NULL, &clkspec);
if (rc)
return NULL;
--
2.54.0
^ permalink raw reply related
* [PATCH v4 6/7] clk: Add support for clock nexus dt bindings
From: Miquel Raynal (Schneider Electric) @ 2026-07-17 15:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, linux-clk,
devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric), Herve Codina
In-Reply-To: <20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com>
A nexus node is some kind of parent device abstracting the outer
connections. They are particularly useful for describing connectors-like
interfaces but not only. Certain IP blocks will typically include inner
blocks and distribute resources to them.
In the case of clocks, there is already the concept of clock controller,
but this usually indicates some kind of control over the said clock,
ie. gate or rate control. When there is none of this, an existing
approach is to reference the upper clock, which is wrong from a hardware
point of view.
Nexus nodes are already part of the device-tree specification and clocks
are already mentioned:
https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping
Following the introductions of nexus nodes support for interrupts, gpios
and pwms, here is the same logic applied again to the clk subsystem,
just by transitioning from of_parse_phandle_with_args() to
of_parse_phandle_with_args_map():
* Nexus OF support:
commit bd6f2fd5a1d5 ("of: Support parsing phandle argument lists through a nexus node")
* GPIO adoption:
commit c11e6f0f04db ("gpio: Support gpio nexus dt bindings")
* PWM adoption:
commit e71e46a6f19c ("pwm: Add support for pwm nexus dt bindings")
Only expected Nexus property:
- clock-map: maps inner clocks to inlet clocks
(the other properties have been judged not relevant for clocks)
Here is an example:
Example:
soc_clk: clock-controller {
#clock-cells = <1>;
};
container: container {
#clock-cells = <1>;
clock-map = <0 &soc_clk 2>,
<1 &soc_clk 6>;
child-device {
clocks = <&container 1>;
/* This is equivalent to <&soc_clk 6> */
};
};
The child device does not need to know about the outer implementation,
and only knows about what the nexus provides. The nexus acts as a
pass-through, with no extra control.
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
---
drivers/clk/clk-conf.c | 12 ++++++------
drivers/clk/clk.c | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
index 303a0bb26e54..5380d43b56a4 100644
--- a/drivers/clk/clk-conf.c
+++ b/drivers/clk/clk-conf.c
@@ -25,8 +25,8 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
node);
for (index = 0; index < num_parents; index++) {
- rc = of_parse_phandle_with_args(node, "assigned-clock-parents",
- "#clock-cells", index, &clkspec);
+ rc = of_parse_phandle_with_args_map(node, "assigned-clock-parents",
+ "clock", index, &clkspec);
if (rc < 0) {
/* skip empty (null) phandles */
if (rc == -ENOENT)
@@ -47,8 +47,8 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
return PTR_ERR(pclk);
}
- rc = of_parse_phandle_with_args(node, "assigned-clocks",
- "#clock-cells", index, &clkspec);
+ rc = of_parse_phandle_with_args_map(node, "assigned-clocks",
+ "clock", index, &clkspec);
if (rc < 0)
goto err;
if (clkspec.np == node && !clk_supplier) {
@@ -121,8 +121,8 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
rate = rates[index];
if (rate) {
- rc = of_parse_phandle_with_args(node, "assigned-clocks",
- "#clock-cells", index, &clkspec);
+ rc = of_parse_phandle_with_args_map(node, "assigned-clocks",
+ "clock", index, &clkspec);
if (rc < 0) {
/* skip empty (null) phandles */
if (rc == -ENOENT)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 45f5d7a4ccc1..6acf042673c3 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -5207,8 +5207,8 @@ static int of_parse_clkspec(const struct device_node *np, int index,
*/
if (name)
index = of_property_match_string(np, "clock-names", name);
- ret = of_parse_phandle_with_args(np, "clocks", "#clock-cells",
- index, out_args);
+ ret = of_parse_phandle_with_args_map(np, "clocks", "clock",
+ index, out_args);
if (!ret)
break;
if (name && index >= 0)
--
2.54.0
^ permalink raw reply related
* [PATCH v4 7/7] clk: tests: Add Kunit testing for nexus nodes
From: Miquel Raynal (Schneider Electric) @ 2026-07-17 15:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, linux-clk,
devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric)
In-Reply-To: <20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com>
Add a nexus node with a child requesting a mapped clock in the fake DT
overlay to verify that the parsing is also correctly working.
Create an of_find_node_by_name() like kunit helper to garbage collect the
node automatically in case of failed assertion.
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
---
drivers/clk/clk_kunit_helpers.c | 31 +++++++++++++++++++++++++++++++
drivers/clk/clk_test.c | 15 +++++++++++++++
drivers/clk/kunit_clk_parse_clkspec.dtso | 10 ++++++++++
include/kunit/clk.h | 2 ++
4 files changed, 58 insertions(+)
diff --git a/drivers/clk/clk_kunit_helpers.c b/drivers/clk/clk_kunit_helpers.c
index 68a28e70bb61..daaf1cf1546c 100644
--- a/drivers/clk/clk_kunit_helpers.c
+++ b/drivers/clk/clk_kunit_helpers.c
@@ -233,5 +233,36 @@ int of_clk_add_hw_provider_kunit(struct kunit *test, struct device_node *np,
}
EXPORT_SYMBOL_GPL(of_clk_add_hw_provider_kunit);
+KUNIT_DEFINE_ACTION_WRAPPER(of_node_put_wrapper, of_node_put, struct device_node *);
+
+/**
+ * of_find_node_by_name_kunit() - Test managed of_find_node_by_name()
+ * @test: The test context
+ * @from: Parent device node to start searching from, or NULL to search from root
+ * @name: The name string to match against
+ *
+ * Just like of_find_node_by_name(), except the device_noded is managed by
+ * the test case and is automatically put after the test case concludes.
+ *
+ * Return: the device_node on success, NULL if not found, or a negative errno value on failure.
+ */
+struct device_node *of_find_node_by_name_kunit(struct kunit *test, struct device_node *from,
+ const char *name)
+{
+ struct device_node *np;
+ int ret;
+
+ np = of_find_node_by_name(from, name);
+ if (!np)
+ return NULL;
+
+ ret = kunit_add_action_or_reset(test, of_node_put_wrapper, np);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return np;
+}
+EXPORT_SYMBOL_GPL(of_find_node_by_name_kunit);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("KUnit helpers for clk providers and consumers");
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index 4084559e157d..0c2f1efdaa8e 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3650,10 +3650,25 @@ static void of_clk_get_parent_name_gets_parent_name(struct kunit *test)
of_clk_get_parent_name(ctx->cons_np, 0));
}
+static void of_clk_get_hw_maps_thru_nexus(struct kunit *test)
+{
+ struct clk_parse_clkspec_ctx *ctx = test->priv;
+ struct clk_hw *expected;
+ struct device_node *np;
+
+ np = of_find_node_by_name_kunit(test, NULL, "kunit-clock-nexus-child");
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np);
+ expected = of_clk_get_hw(ctx->cons_np, 1, NULL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, expected);
+
+ KUNIT_EXPECT_PTR_EQ(test, expected, of_clk_get_hw(np, 0, NULL));
+}
+
static struct kunit_case clk_parse_clkspec_test_cases[] = {
KUNIT_CASE(clk_parse_clkspec_with_correct_index_and_name),
KUNIT_CASE(clk_parse_clkspec_with_incorrect_index_and_name),
KUNIT_CASE(of_clk_get_parent_name_gets_parent_name),
+ KUNIT_CASE(of_clk_get_hw_maps_thru_nexus),
{}
};
diff --git a/drivers/clk/kunit_clk_parse_clkspec.dtso b/drivers/clk/kunit_clk_parse_clkspec.dtso
index c93feb93e101..a4115216d2aa 100644
--- a/drivers/clk/kunit_clk_parse_clkspec.dtso
+++ b/drivers/clk/kunit_clk_parse_clkspec.dtso
@@ -18,4 +18,14 @@ kunit-clock-consumer {
clocks = <&kunit_clock_provider1 0>, <&kunit_clock_provider2 0>;
clock-names = "first_clock", "second_clock";
};
+
+ kunit_clock_nexus: kunit-clock-nexus {
+ clocks = <&kunit_clock_provider2 0>;
+ clock-map = <&kunit_clock_provider2 0>;
+ #clock-cells = <0>;
+
+ kunit-clock-nexus-child {
+ clocks = <&kunit_clock_nexus>;
+ };
+ };
};
diff --git a/include/kunit/clk.h b/include/kunit/clk.h
index f226044cc78d..ab24f7747516 100644
--- a/include/kunit/clk.h
+++ b/include/kunit/clk.h
@@ -29,5 +29,7 @@ int of_clk_hw_register_kunit(struct kunit *test, struct device_node *node,
int of_clk_add_hw_provider_kunit(struct kunit *test, struct device_node *np,
struct clk_hw *(*get)(struct of_phandle_args *clkspec, void *data),
void *data);
+struct device_node *of_find_node_by_name_kunit(struct kunit *test, struct device_node *from,
+ const char *name);
#endif
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] dma-iommu: Introduce API to reserve IOVA regions for dynamically created devices
From: Jason Gunthorpe @ 2026-07-17 16:07 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Krzysztof Kozlowski, Vishnu Reddy, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, devicetree, Vikash Garodia,
Robin Murphy, joro, will, m.szyprowski, iommu, linux-kernel,
dikshita.agarwal, Bjorn Andersson, Dmitry Baryshkov, Robin Clark,
Akhil P Oommen, Srinivas Kandagatla, Ekansh Gupta, Loic Poulain
In-Reply-To: <dba037fa-0f21-4a50-8727-076a3b177c47@gmail.com>
On Fri, Jul 17, 2026 at 12:44:54PM +0200, Konrad Dybcio wrote:
> I was kinda hoping there would be more buy-in to follow what PCIe
> does, but to my understanding you're opposed to it because "vpu_bus"
> is not dynamically discoverable.
PCIe allows describing devices in DT. What this weird vpu_bus thing
seems to be about is to actively avoid describing the devices in DT
while simultaenously treating them like devices. I think it makes no
sense..
IMHO the approach inverts the thing, you start the main driver then
use vpu_bus to dynamically create a bunch of struct device DMA handles
for the main driver to use.
I think it is much better if you start the main driver, use the new
component stuff:
https://docs.kernel.org/driver-api/component.html
To aggregate a driver from the struct device DMA handles that DT will
create automatically. ie the reversed flow, the struct devices
pre-exist created by DT instead of being created by code in your
driver.
Jason
^ permalink raw reply
* Re: [PATCH v28 3/7] firmware: imx: add driver for NXP EdgeLock Enclave
From: Frank Li @ 2026-07-17 16:11 UTC (permalink / raw)
To: sashiko-reviews; +Cc: pankaj.gupta, Frank.Li, imx, devicetree, conor+dt, robh
In-Reply-To: <20260717111009.B55531F000E9@smtp.kernel.org>
On Fri, Jul 17, 2026 at 11:10:09AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
> - [High] The mailbox receive callback `se_if_rx_callback` dereferences `msg` without checking `IS_ERR()`, leading to a kernel panic if the mailbox controller returns an error.
> - [High] `ele_msg_rcv()` uses `MAX_SCHEDULE_TIMEOUT` causing an infinite uninterruptible wait and subsystem deadlock if the firmware hangs.
> - [Medium] Global state `var_se_info.soc_rev` prevents SoC device registration and per-device IMEM state initialization on driver rebind or secondary device instances.
> - [Medium] Casting a 1-byte aligned stack array to `struct ele_dev_info` and `u32` pointers causes undefined behavior and potential unaligned memory access faults.
> - [Medium] The driver fails to expose the file descriptor interfaces claimed in the commit message and Kconfig.
> - [Low] The SoC revision is incorrectly formatted as minor.major instead of major.minor.
> --
>
> --- Patch [3]: [PATCH v28 3/7] firmware: imx: add driver for NXP EdgeLock Enclave ---
> commit a06a0c9a5464457c1ae77bb60350c41fff331ed9
> Author: Pankaj Gupta <pankaj.gupta@nxp.com>
> Subject: firmware: imx: add driver for NXP EdgeLock Enclave
>
> Add MU-based communication interface for secure enclave.
> The secure enclave FW communicates with Linux over single or multiple
> dedicated messaging unit(MU) based interface(s).
>
> Link: https://sashiko.dev/#/patchset/20260715-imx-se-if-v27-0-bb7c45952f06@nxp.com?part=3
>
> > diff --git a/drivers/firmware/imx/Kconfig b/drivers/firmware/imx/Kconfig
> > index 127ad752acf86..e3cb7f965e70e 100644
> > --- a/drivers/firmware/imx/Kconfig
> > +++ b/drivers/firmware/imx/Kconfig
>
> [ ... ]
>
> > @@ -55,3 +55,16 @@ config IMX_SCMI_MISC_DRV
> > core that could provide misc functions such as board control.
> >
> > This driver can also be built as a module.
> > +
> > +config IMX_SEC_ENCLAVE
> > + tristate "i.MX Embedded Secure Enclave - EdgeLock Enclave Firmware driver."
> > + depends on MAILBOX && ((IMX_MBOX && ARCH_MXC && ARM64) || COMPILE_TEST)
> > + select FW_LOADER
> > + default m if ARCH_MXC
> > +
> > + help
> > + Exposes APIs supported by the iMX Secure Enclave HW IP called:
> > + - EdgeLock Enclave Firmware (for i.MX8ULP, i.MX93),
> > + like base, HSM, V2X & SHE using the SAB protocol via the shared Messaging
> > + Unit. This driver exposes these interfaces via a set of file descriptors
> > + allowing to configure shared memory, send and receive messages.
>
> [Severity: Medium]
> Is this Kconfig description accurate? It mentions exposing interfaces via a
> set of file descriptors allowing to configure shared memory, send and receive
> messages. However, there is no character device registration or file
> operations implemented in this patch.
please check this
>
> > diff --git a/drivers/firmware/imx/ele_base_msg.h b/drivers/firmware/imx/ele_base_msg.h
> > new file mode 100644
> > index 0000000000000..4c3699543e879
> > --- /dev/null
> > +++ b/drivers/firmware/imx/ele_base_msg.h
>
> [ ... ]
>
> > @@ -0,0 +1,98 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright 2025 NXP
> > + *
> > + * Header file for the EdgeLock Enclave Base API(s).
> > + */
>
> [ ... ]
>
> > +#define GET_SERIAL_NUM_FROM_UID(x, uid_word_sz) ({\
> > + const u32 *__x = (const u32 *)(x); \
> > + size_t __sz = (uid_word_sz); \
> > + ((u64)__x[__sz - 1] << 32) | __x[0]; \
> > + })
>
> [Severity: Medium]
> Can this macro cause undefined behavior? When called from get_se_soc_info(),
> the x parameter points into a 1-byte aligned stack array (u8 data). Casting
> this to a u32 pointer and dereferencing it could trigger unaligned memory
> access faults on architectures with strict alignment requirements.
>
> > diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
> > new file mode 100644
> > index 0000000000000..ca8a32a82689b
> > --- /dev/null
> > +++ b/drivers/firmware/imx/ele_common.c
>
> [ ... ]
>
> > @@ -0,0 +1,470 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2025 NXP
> > + */
>
> [ ... ]
>
> > +int ele_msg_rcv(struct se_if_priv *priv, struct se_clbk_handle *se_clbk_hdl)
> > +{
> > + bool wait_uninterruptible = false;
> > + unsigned long remaining_jiffies;
> > + unsigned long flags;
> > + int ret;
> > +
> > + remaining_jiffies = MAX_SCHEDULE_TIMEOUT;
> > + do {
> > + if (wait_uninterruptible)
> > + ret = wait_for_completion_timeout(&se_clbk_hdl->done,
> > + remaining_jiffies);
>
> [Severity: High]
> Can this loop cause a subsystem deadlock? Since remaining_jiffies is set to
> MAX_SCHEDULE_TIMEOUT, wait_for_completion_timeout() blocks indefinitely. If
> the firmware hangs and wait_uninterruptible is set to true after a signal,
> the thread will enter an infinite uninterruptible sleep while holding the
> se_if_cmd_lock mutex. The timeout fallback and circuit breaker logic become
> unreachable.
Can you use reasonable timeout value to avoid deadlock?
>
> > + else
> > + ret = wait_for_completion_interruptible_timeout(&se_clbk_hdl->done,
> > + remaining_jiffies);
>
> [ ... ]
>
> > +void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
> > +{
> > + struct se_clbk_handle *se_clbk_hdl;
> > + struct device *dev = mbox_cl->dev;
> > + struct se_msg_hdr *header;
> > + bool sz_mismatch = false;
> > + struct se_if_priv *priv;
> > + unsigned long flags;
> > + u32 rx_msg_sz;
> > +
> > + priv = dev_get_drvdata(dev);
> > +
> > + /* The function can be called with NULL msg */
> > + if (!msg) {
> > + dev_err(dev, "Message is invalid\n");
> > + return;
> > + }
> > +
> > + header = msg;
> > + rx_msg_sz = header->size << 2;
>
> [Severity: High]
> Does this code safely handle mailbox errors? If the mailbox controller
> returns an error (like a timeout), it typically passes ERR_PTR(err) to the
> callback. The check for !msg allows ERR_PTR to bypass the check, which
> would lead to a kernel panic when msg is cast to struct se_msg_hdr and
> dereferenced here.
Please double check this ? if yes, should use IS_ERR()
>
> > diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> > new file mode 100644
> > index 0000000000000..9bb59c200911b
> > --- /dev/null
> > +++ b/drivers/firmware/imx/se_ctrl.c
>
> [ ... ]
>
> > @@ -0,0 +1,417 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2026 NXP
> > + */
>
> [ ... ]
>
> > +static int get_se_soc_info(struct se_if_priv *priv, const struct se_soc_info *se_info)
> > +{
> > + struct se_fw_load_info *load_fw = get_load_fw_instance(priv);
> > + struct soc_device_attribute *attr;
> > + u8 data[MAX_SOC_INFO_DATA_SZ];
> > + struct ele_dev_info *s_info;
> > + struct soc_device *sdev;
> > + int err = 0;
> > +
> > + /*
> > + * This function should be called once.
> > + * Check if the se_soc_rev is zero to continue.
> > + */
> > + guard(mutex)(&var_se_info.se_var_info_lock);
> > + if (var_se_info.soc_rev)
> > + return err;
>
> [Severity: Medium]
> Will this early return prevent initialization on driver rebind? When the
> driver is unbound, soc_device_unregister() removes the SoC device, but the
> global var_se_info.soc_rev is never cleared. On a subsequent bind, a
> non-zero soc_rev causes an early return, bypassing soc_device_register()
> and leaving load_fw->imem.state uninitialized.
>
> > +
> > + err = ele_fetch_soc_info(priv, &data);
>
> [ ... ]
>
> > + if (FIELD_GET(DEV_GETINFO_MIN_VER_MASK, var_se_info.soc_rev))
> > + attr->revision = devm_kasprintf(priv->dev, GFP_KERNEL, "%x.%x",
> > + FIELD_GET(DEV_GETINFO_MIN_VER_MASK,
> > + var_se_info.soc_rev),
> > + FIELD_GET(DEV_GETINFO_MAJ_VER_MASK,
> > + var_se_info.soc_rev));
>
> [Severity: Low]
> Is this string format correct? It prints MIN_VER_MASK before MAJ_VER_MASK,
> resulting in minor.major instead of major.minor. This might break userspace
> parsing expectations.
Make sense, please fix it.
Frank
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260717-imx-se-if-v28-0-0a9659c7e69d@nxp.com?part=3
^ permalink raw reply
* Re: [PATCH v28 4/7] firmware: imx: device context dedicated to priv
From: Frank Li @ 2026-07-17 16:14 UTC (permalink / raw)
To: sashiko-reviews; +Cc: pankaj.gupta, Frank.Li, robh, conor+dt, devicetree, imx
In-Reply-To: <20260717111353.7D5471F000E9@smtp.kernel.org>
On Fri, Jul 17, 2026 at 11:13:53AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Data race in `ele_msg_send_rcv()` when initializing `waiting_rsp_clbk_hdl` state, leading to a potential NULL pointer dereference in the RX interrupt handler.
> - [Low] Unused variable `flags` in `ele_msg_send_rcv()`.
> --
>
> --- Patch [4]: [PATCH v28 4/7] firmware: imx: device context dedicated to priv ---
> commit 64e554919e156aa88c2007ef9dd9fafde735e583
> Author: Pankaj Gupta <pankaj.gupta@nxp.com>
>
> firmware: imx: device context dedicated to priv
>
> Add priv_dev_ctx to prepare enabling misc-device context based send-receive
> path, to communicate with FW. No functionality change.
>
> Closes: https://sashiko.dev/#/patchset/20260715-imx-se-if-v27-0-bb7c45952f06@nxp.com?part=4
>
> > diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
> > index ca8a32a82689b..1ca22c53b55c4 100644
> > --- a/drivers/firmware/imx/ele_common.c
> > +++ b/drivers/firmware/imx/ele_common.c
>
> [ ... ]
>
> > @@ -149,34 +151,38 @@ static void ele_msg_send_rcv_cleanup(struct se_if_priv *priv)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock, flags);
> > + priv->waiting_rsp_clbk_hdl.dev_ctx = NULL;
> > priv->waiting_rsp_clbk_hdl.rx_msg = NULL;
> > priv->waiting_rsp_clbk_hdl.rx_msg_sz = 0;
> > spin_unlock_irqrestore(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock, flags);
> > }
> >
> > /* API used for send/receive blocking call. */
> > -int ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, int tx_msg_sz,
> > - void *rx_msg, int exp_rx_msg_sz)
> > +int ele_msg_send_rcv(struct se_if_device_ctx *dev_ctx, void *tx_msg,
> > + int tx_msg_sz, void *rx_msg, int exp_rx_msg_sz)
> > {
> > + struct se_if_priv *priv = dev_ctx->priv;
> > + unsigned long flags;
>
> [Severity: Low]
> Is this flags variable used anywhere in this function?
>
> It appears to be declared but never utilized in the function scope, which
> will trigger a -Wunused-variable compiler warning.
Yes, fix it.
>
> > int err;
> >
> > guard(mutex)(&priv->se_if_cmd_lock);
> >
> > if (atomic_read(&priv->fw_busy)) {
> > - dev_dbg(priv->dev, "ELE became unresponsive.\n");
> > + dev_dbg(priv->dev, "%s: ELE became unresponsive.\n", dev_ctx->devname);
> > return -EBUSY;
> > }
> > reinit_completion(&priv->waiting_rsp_clbk_hdl.done);
> > + priv->waiting_rsp_clbk_hdl.dev_ctx = dev_ctx;
> > priv->waiting_rsp_clbk_hdl.rx_msg_sz = exp_rx_msg_sz;
> > priv->waiting_rsp_clbk_hdl.rx_msg = rx_msg;
>
> [Severity: High]
> Could these lockless updates result in a data race with the interrupt
> handler?
>
> They are updated here without acquiring clbk_rx_lock. If a delayed firmware
> response from a previous command arrives concurrently, the interrupt handler
> se_if_rx_callback() will execute.
>
> Due to CPU reordering or compiler optimizations, can the interrupt handler
> observe the new rx_msg pointer before the new dev_ctx pointer becomes
> visible?
Please check this
Frank
>
> > - err = ele_msg_send(priv, tx_msg, tx_msg_sz);
> > + err = ele_msg_send(dev_ctx, tx_msg, tx_msg_sz);
>
> [ ... ]
>
> > @@ -302,8 +310,8 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
> > dev_info(dev, "ELE responded (late), recovery FW available.");
> > return;
> > }
> > - dev_dbg(dev, "Selecting resp waiter: for mesg header:0x%x.",
> > - *(u32 *)header);
> > + dev_dbg(dev, "Selecting resp waiter:%s for mesg header:0x%x.",
> > + se_clbk_hdl->dev_ctx->devname, *(u32 *)header);
>
> If the interrupt handler sees a non-NULL rx_msg, it assumes the transaction
> is active and attempts to access se_clbk_hdl->dev_ctx->devname here. If
> dev_ctx is still NULL from the previous cleanup, won't this result in a NULL
> pointer dereference?
>
> >
> > /*
> > * For rsp_tag traffic, the sender provides the expected response
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260717-imx-se-if-v28-0-0a9659c7e69d@nxp.com?part=4
^ permalink raw reply
* Re: [PATCH v28 5/7] firmware: drivers: imx: adds miscdev
From: Frank Li @ 2026-07-17 16:26 UTC (permalink / raw)
To: sashiko-reviews; +Cc: pankaj.gupta, conor+dt, imx, devicetree, robh, Frank.Li
In-Reply-To: <20260717111257.260351F000E9@smtp.kernel.org>
On Fri, Jul 17, 2026 at 11:12:55AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 13 potential issue(s) to consider:
>
> New issues:
> - [Critical] ele_get_info() uses an uninitialized variable for memory size and fails to check for allocation failure.
> - [High] Unconditional and lockless clearing of rx_msg_sz in se_if_fops_read causes state corruption and message loss.
> - [High] Returning -ERESTARTSYS after a successful firmware transaction causes non-idempotent commands to be re-executed and discards successful responses.
> - [High] se_if_fops_read() performs shared memory cleanup without holding the fops_lock, leading to double-free and list corruption.
> - [High] Mailbox channels are freed before the misc device is deregistered due to inverted devres ordering, allowing use-after-free on channels.
> - [High] TOCTOU race between se_if_fops_open and se_if_probe_cleanup allows a device context to escape teardown and access freed hardware resources.
> - [High] init_misc_device_context() leaks DMA memory on error paths and falsely reports success if gate allocation fails.
> - [High] The DMA coherent memory allocated for priv_dev_ctx is never freed during driver teardown.
> - [High] ele_get_info_cleanup() frees gen_pool memory immediately on timeout, allowing the firmware to corrupt reallocated memory.
> - [High] Data race and memory corruption in shared memory allocator get_shared_mem_slot() for priv_dev_ctx.
> - [High] TOCTOU race in se_if_fops_read allows dereferencing a freed rx_msg buffer.
> - [High] The misc device is registered before driver probe completes, exposing uninitialized state.
>
> Pre-existing issues:
> - [High] User IOCTL interrupted by a signal can trigger the hung-task watchdog due to an excessive 5000-second uninterruptible sleep.
> --
>
> --- Patch [5]: [PATCH v28 5/7] firmware: drivers: imx: adds miscdev ---
> commit 65c691a4b86b3b1a94b7ed63800d73fff3355b12
> Author: Pankaj Gupta <pankaj.gupta@nxp.com>
>
> firmware: drivers: imx: adds miscdev
>
> Adds the driver for communication interface to secure-enclave, enabling
> userspace applications via a character driver. It implements the ABI for
> shared memory management, command sending, and file operations.
>
> > diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
> > --- a/drivers/firmware/imx/ele_base_msg.c
> > +++ b/drivers/firmware/imx/ele_base_msg.c
> > @@ -32,6 +32,9 @@ static void ele_get_info_cleanup(struct se_if_priv *priv, u32 *buf, dma_addr_t d
> > size_t size)
> > {
> > if (priv->mem_pool)
> > gen_pool_free(priv->mem_pool, (unsigned long)buf, size);
>
> [Severity: High]
> Does this unconditionally free the DMA buffer when priv->mem_pool is true,
> even if the firmware is still busy processing a timed-out command?
>
> If the enclave command times out, the firmware might still perform delayed
> DMA writes to this buffer. Releasing it immediately allows the memory to be
> reallocated while the hardware is still accessing it, potentially leading
> to memory corruption.
This is hard to fix. DMA may still runnings and can't sync with cleanup
function. You can add comments here.
If timeout value is bigger enough, it is statistically impossible
>
> [ ... ]
>
> > @@ -74,9 +74,9 @@ int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
> >
> > if (priv->mem_pool) {
> > get_info_data = gen_pool_dma_alloc(priv->mem_pool,
> > ELE_GET_INFO_BUFF_SZ,
> > &get_info_addr);
> > } else {
>
> [Severity: Critical]
> Does this leave get_info_len uninitialized and skip checking if get_info_data
> is NULL when priv->mem_pool is true?
>
> If gen_pool_dma_alloc() fails, the code continues and calls memcpy() on a
> NULL pointer.
This need check return value.
>
> Further down, it passes the uninitialized get_info_len to
> ele_get_info_cleanup(), which would cause gen_pool_free() to corrupt the
> allocator state by freeing a random size.
>
> [ ... ]
>
> > diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
> > --- a/drivers/firmware/imx/ele_common.c
> > +++ b/drivers/firmware/imx/ele_common.c
> > @@ -107,6 +107,10 @@ int ele_msg_rcv(struct se_if_device_ctx *dev_ctx, struct se_clbk_handle *se_clbk
> > if (wait_uninterruptible)
> > ret = wait_for_completion_timeout(&se_clbk_hdl->done,
> > remaining_jiffies);
>
> [Severity: High]
> This is a pre-existing issue, but can this trigger the hung-task watchdog
> when a user IOCTL is interrupted by a signal?
>
> The userspace contexts are assigned a timeout of 5,000,000 ms in
> init_device_context().
>
> If wait_for_completion_interruptible_timeout() returns -ERESTARTSYS, it
> switches to wait_uninterruptible for the remainder of that 5000-second
> timeout. If the hardware does not respond, this uninterruptible sleep will
> exceed the 120-second kernel hung-task threshold and panic the system.
>
> [ ... ]
>
> > @@ -216,6 +216,8 @@ int ele_msg_send_rcv(struct se_if_device_ctx *dev_ctx, void *tx_msg,
> > int tx_msg_sz, void *rx_msg, int exp_rx_msg_sz)
> > {
> [ ... ]
> > + if (err > 0)
> > + err = -ERESTARTSYS;
>
> [Severity: High]
> Does returning -ERESTARTSYS here discard a successful firmware response
> and cause non-idempotent commands to be re-executed?
>
> If a signal is received but the firmware transaction still completes
> successfully, the success code (bytes received) appears to be overwritten
> with -ERESTARTSYS near the end of this function.
>
> This would cause the kernel to auto-restart the IOCTL, re-executing the
> command on the hardware, which could corrupt state for operations like key
> generation or lifecycle changes.
doube check this one
>
> [ ... ]
>
> > diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> > --- a/drivers/firmware/imx/se_ctrl.c
> > +++ b/drivers/firmware/imx/se_ctrl.c
> > @@ -487,14 +487,18 @@ static int init_misc_device_context(struct se_if_priv *priv, int ch_id,
> > gate = kzalloc_obj(*gate, GFP_KERNEL);
> > if (!gate)
> > goto exit;
> >
> > [ ... ]
> > exit:
> > *new_dev_ctx = NULL;
> > if (gate) {
> > priv->open_gate = NULL;
> > se_if_open_gate_put(gate);
> > }
> > kfree(dev_ctx->devname);
> > kfree(dev_ctx);
> > return dev_err_probe(priv->dev, ret, "%s", err_str);
> > }
>
> [Severity: High]
> Can this leak DMA memory and falsely report success if the gate allocation
> fails?
>
> If kzalloc_obj() for gate fails, the code jumps to the exit label where
> ret is still 0 (from the successful init_se_shared_mem() call).
Yes, need set ret = -ENOMEN
>
> The function returns 0, leading se_if_probe() to interpret it as a
> success, but the 128KB DMA buffer allocated for the shared memory is
> never cleaned up. Subsequent driver operations might then trigger a NULL
> pointer dereference since priv_dev_ctx is NULL.
>
> [ ... ]
>
> > @@ -531,10 +531,9 @@ static void se_if_priv_release(struct kref *kref)
> > if (priv->priv_dev_ctx) {
> > kfree(priv->priv_dev_ctx->devname);
> > kfree(priv->priv_dev_ctx);
> > priv->priv_dev_ctx = NULL;
> > }
>
> [Severity: High]
> Is the DMA coherent memory allocated for priv_dev_ctx leaked here?
>
> During probe, init_misc_device_context() allocates 128KB of DMA shared
> memory for priv_dev_ctx. When se_if_priv_release() frees the structure,
> it seems to skip calling cleanup_se_shared_mem(), permanently leaking
> this memory whenever the module is unloaded or probe fails.
most likely correct.
>
> [ ... ]
>
> > @@ -993,6 +993,8 @@ int get_shared_mem_slot(struct se_if_device_ctx *dev_ctx,
> > /* Allocate space in shared memory. 8 bytes aligned. */
> > pos = shared_mem->pos;
> > shared_mem->pos += aligned_len;
>
> [Severity: High]
> Can this lockless increment of shared_mem->pos cause a data race and
> memory corruption when called for the internal priv_dev_ctx?
>
> The userspace IOCTLs protect this via dev_ctx->fops_lock, but kernel
> threads calling ele_get_info() on priv->priv_dev_ctx do not acquire this
> lock.
make sense, please check it.
Anyways, it is much better than before. Please continue follow sashiko
comments.
Frank
>
> If a concurrent operation (like se_load_firmware) happens at the
> same time as ele_get_info, both threads could modify pos simultaneously,
> causing overlapping DMA memory allocations and payload corruption.
>
> [ ... ]
>
> > @@ -1180,6 +1180,9 @@ static ssize_t se_if_fops_read(struct file *fp, char __user *buf, size_t size,
> > if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx) {
> > priv->cmd_receiver_clbk_hdl.rx_msg_sz = 0;
> > se_dev_ctx_shared_mem_cleanup(dev_ctx);
> > return -EINVAL;
> > }
>
> [Severity: High]
> Does this allow unprivileged processes to locklessly corrupt the command
> receiver's state?
>
> If the calling dev_ctx is not the registered command receiver,
> rx_msg_sz is cleared without holding clbk_rx_lock. This allows any
> process capable of calling read() to disrupt the NVM daemon's state.
>
> [ ... ]
>
> > @@ -1222,6 +1222,8 @@ static ssize_t se_if_fops_read(struct file *fp, char __user *buf, size_t size,
> > spin_unlock_irqrestore(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock, flags);
> >
> > /* We may need to copy the output data to user before
> > * delivering the completion message.
> > */
> > err = se_dev_ctx_cpy_out_data(dev_ctx);
> > if (err < 0)
> > goto exit_mutex_unlock;
> >
> > /* Copy data from the buffer */
> > print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
> > priv->cmd_receiver_clbk_hdl.rx_msg,
> > priv->cmd_receiver_clbk_hdl.rx_msg_sz,
> > false);
> >
> > copy_len = min(size, priv->cmd_receiver_clbk_hdl.rx_msg_sz);
> >
> > if (copy_to_user(buf, priv->cmd_receiver_clbk_hdl.rx_msg, copy_len))
>
> [Severity: High]
> Can this lead to a use-after-free of the rx_msg buffer?
>
> The rx_msg pointer is validated while holding clbk_rx_lock, but the lock
> is dropped before passing rx_msg to copy_to_user().
>
> If driver unbind runs concurrently, unset_dev_ctx_as_command_receiver()
> will acquire the lock, set rx_msg to NULL, and immediately free the buffer.
> The read() path will then copy freed kernel memory to userspace.
>
> [ ... ]
>
> > @@ -1248,6 +1248,8 @@ exit_mutex_unlock:
> > mutex_unlock(&dev_ctx->fops_lock);
> >
> > exit:
> > spin_lock_irqsave(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock, flags);
> > priv->cmd_receiver_clbk_hdl.rx_msg_sz = 0;
> > spin_unlock_irqrestore(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock, flags);
> > se_dev_ctx_shared_mem_cleanup(dev_ctx);
>
> [Severity: High]
> Does this unconditionally clear rx_msg_sz and perform lockless shared memory
> cleanup on the exit path?
>
> If ele_msg_rcv() is interrupted by a signal, rx_msg_sz is cleared, which
> will permanently drop a concurrently arriving message by overwriting its
> size with 0.
>
> Additionally, se_dev_ctx_shared_mem_cleanup() modifies the pending lists
> and frees their entries without holding fops_lock. If se_if_fops_close()
> runs concurrently (e.g. process termination), it can cause list corruption
> and double-frees.
>
> [ ... ]
>
> > @@ -1282,10 +1282,11 @@ static int se_if_fops_open(struct inode *nd, struct file *fp)
> > priv = gate->priv;
> > mutex_unlock(&gate->lock);
> >
> > misc_dev_ctx = priv->priv_dev_ctx;
> >
> > if (mutex_lock_interruptible(&misc_dev_ctx->fops_lock)) {
> > err = -EBUSY;
> > goto out_put_priv;
> > }
> >
> > if (misc_dev_ctx->cleanup_done) {
> > err = -ENODEV;
> > goto out_unlock_misc;
> > }
> >
> > priv->dev_ctx_mono_count++;
> > err = init_device_context(priv, priv->dev_ctx_mono_count, &dev_ctx);
>
> [Severity: High]
> Is there a race condition here that allows a new context to escape teardown?
>
> By dropping gate->lock before creating and adding the new device context
> to priv->dev_ctx_list, se_if_probe_cleanup() can concurrently drain all
> existing contexts.
>
> After teardown finishes, this function will append the newly allocated
> context to the list and return a valid fd to userspace. This would allow
> userspace to access hardware resources like mailbox channels that have
> already been destroyed by devres.
>
> [ ... ]
>
> > @@ -1546,6 +1546,9 @@ static int se_if_probe(struct platform_device *pdev)
> >
> > ret = devm_add_action_or_reset(dev, se_if_probe_cleanup, pdev);
> > if (ret)
> > return ret;
> >
> > /* Mailbox client configuration */
> > priv->se_mb_cl.dev = dev;
> > priv->se_mb_cl.tx_block = false;
> > priv->se_mb_cl.knows_txdone = false;
> > priv->se_mb_cl.rx_callback = se_if_rx_callback;
> >
> > ret = se_if_request_channel(dev, &priv->tx_chan, &priv->se_mb_cl, MBOX_TX_NAME);
>
> [Severity: High]
> Will devres free the mailbox channels before deregistering the misc device?
>
> Because se_if_probe_cleanup is registered as a devres action before the
> mailbox channels are requested, it will run after the channels are freed
> (LIFO order).
>
> During this window, the character device is still active. If userspace
> issues a write() or IOCTL, the driver will use the freed channel pointers,
> resulting in a use-after-free.
>
> [ ... ]
>
> > @@ -1588,6 +1588,11 @@ static int se_if_probe(struct platform_device *pdev)
> > ret = init_misc_device_context(priv, 0, &priv->priv_dev_ctx, &se_if_fops);
> > if (ret)
> > return dev_err_probe(dev, ret,
> > "Failed[0x%x] to create device contexts.",
> > ret);
> >
> > if (if_node->if_defs.se_if_type == SE_TYPE_ID_HSM) {
> > ret = get_se_soc_info(priv, se_info);
> > if (ret)
> > return dev_err_probe(dev, ret, "Failed to fetch SoC Info.");
> > }
> >
> > /* By default, there is no pending FW to be loaded.*/
> > if (se_info->se_fw_img_nm.seco_fw_nm_in_rfs) {
> > load_fw = get_load_fw_instance(priv);
> > mutex_init(&load_fw->load_fw_lock);
>
> [Severity: High]
> Is the misc device exposed to userspace before critical state is initialized?
>
> init_misc_device_context() registers the misc device, making it instantly
> available. However, load_fw->load_fw_lock and var_se_info.soc_rev are
> initialized later in the probe sequence.
>
> A fast userspace process could open the device and trigger
> se_load_firmware() via an IOCTL, which would attempt to lock the
> uninitialized load_fw_lock, causing a panic.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260717-imx-se-if-v28-0-0a9659c7e69d@nxp.com?part=5
^ permalink raw reply
* Re: [PATCH 1/3] arm64: dts: imx93-11x11-frdm: factor out common parts into dtsi
From: Frank Li @ 2026-07-17 16:29 UTC (permalink / raw)
To: Joseph Guo
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Shawn Guo,
devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20260717-imx93w_frdm-v1-1-0cca6c0ba2f5@nxp.com>
On Fri, Jul 17, 2026 at 02:11:52PM +0900, Joseph Guo wrote:
> [You don't often get email from qijian.guo@nxp.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> The i.MX93 Wireless FRDM board reuses most of the i.MX93 FRDM board.
> To avoid duplication and DTS-to-DTS includes, extract common hardware
> definitions from imx93-11x11-frdm.dts into imx93-11x11-frdm-common.dtsi
> to allow the common description shared by both boards.
>
> The FRDM-IMX93 board-specific .dts now includes the common dtsi and only
> contains board-specific overrides.
>
> Signed-off-by: Joseph Guo <qijian.guo@nxp.com>
> ---
> .../dts/freescale/imx93-11x11-frdm-common.dtsi | 692 +++++++++++++++++++++
> arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts | 686 +-------------------
> 2 files changed, 694 insertions(+), 684 deletions(-)
Please use -M -C to generate patch. git can auto detect copy.
Frank
>
> diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-frdm-common.dtsi b/arch/arm64/boot/dts/freescale/imx93-11x11-frdm-common.dtsi
> new file mode 100644
> index 0000000000000000000000000000000000000000..69e23570f71f40a3f3517d31e16a9ca1d3dfb1f2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-11x11-frdm-common.dtsi
> @@ -0,0 +1,692 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +#include <dt-bindings/usb/pd.h>
> +
> +/ {
> +
> + aliases {
> + can0 = &flexcan2;
> + ethernet0 = &fec;
> + ethernet1 = &eqos;
> + i2c0 = &lpi2c1;
> + i2c1 = &lpi2c2;
> + i2c2 = &lpi2c3;
> + mmc0 = &usdhc1; /* EMMC */
> + mmc1 = &usdhc2; /* uSD */
> + rtc0 = &pcf2131;
> + serial0 = &lpuart1;
> + serial4 = &lpuart5;
> + };
> +
> + chosen {
> + stdout-path = &lpuart1;
> + };
> +
> + flexcan2_phy: can-phy {
> + compatible = "nxp,tja1051";
> + #phy-cells = <0>;
> + max-bitrate = <5000000>;
> + silent-gpios = <&pcal6524 23 GPIO_ACTIVE_HIGH>;
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + button-k2 {
> + label = "Button K2";
> + linux,code = <BTN_1>;
> + gpios = <&pcal6524 5 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> + interrupt-parent = <&pcal6524>;
> + interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
> + };
> +
> + button-k3 {
> + label = "Button K3";
> + linux,code = <BTN_2>;
> + gpios = <&pcal6524 6 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> + interrupt-parent = <&pcal6524>;
> + interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
> + };
> + };
> +
> + reg_usdhc2_vmmc: regulator-usdhc2 {
> + compatible = "regulator-fixed";
> + off-on-delay-us = <12000>;
> + pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> + pinctrl-names = "default";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "VSD_3V3";
> + vin-supply = <&buck4>;
> + gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + reserved-memory {
> + ranges;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + linux,cma {
> + compatible = "shared-dma-pool";
> + alloc-ranges = <0 0x80000000 0 0x30000000>;
> + reusable;
> + size = <0 0x10000000>;
> + linux,cma-default;
> + };
> +
> + rsc_table: rsc-table@2021e000 {
> + reg = <0 0x2021e000 0 0x1000>;
> + no-map;
> + };
> +
> + vdev0vring0: vdev0vring0@a4000000 {
> + reg = <0 0xa4000000 0 0x8000>;
> + no-map;
> + };
> +
> + vdev0vring1: vdev0vring1@a4008000 {
> + reg = <0 0xa4008000 0 0x8000>;
> + no-map;
> + };
> +
> + vdev1vring0: vdev1vring0@a4010000 {
> + reg = <0 0xa4010000 0 0x8000>;
> + no-map;
> + };
> +
> + vdev1vring1: vdev1vring1@a4018000 {
> + reg = <0 0xa4018000 0 0x8000>;
> + no-map;
> + };
> +
> + vdevbuffer: vdevbuffer@a4020000 {
> + compatible = "shared-dma-pool";
> + reg = <0 0xa4020000 0 0x100000>;
> + no-map;
> + };
> + };
> +};
> +
> +&adc1 {
> + vref-supply = <&buck5>;
> + status = "okay";
> +};
> +
> +&mu1 {
> + status = "okay";
> +};
> +
> +&cm33 {
> + mboxes = <&mu1 0 1>,
> + <&mu1 1 1>,
> + <&mu1 3 1>;
> + mbox-names = "tx", "rx", "rxdb";
> + memory-region = <&vdevbuffer>, <&vdev0vring0>, <&vdev0vring1>,
> + <&vdev1vring0>, <&vdev1vring1>, <&rsc_table>;
> + status = "okay";
> +};
> +
> +&eqos {
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&pinctrl_eqos>;
> + pinctrl-1 = <&pinctrl_eqos_sleep>;
> + phy-handle = <ðphy1>;
> + phy-mode = "rgmii-id";
> + status = "okay";
> +
> + mdio {
> + compatible = "snps,dwmac-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + clock-frequency = <5000000>;
> +
> + ethphy1: ethernet-phy@1 {
> + reg = <1>;
> + reset-assert-us = <10000>;
> + reset-deassert-us = <80000>;
> + reset-gpios = <&pcal6524 15 GPIO_ACTIVE_LOW>;
> + realtek,clkout-disable;
> + };
> + };
> +};
> +
> +&fec {
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&pinctrl_fec>;
> + pinctrl-1 = <&pinctrl_fec_sleep>;
> + phy-mode = "rgmii-id";
> + phy-handle = <ðphy2>;
> + fsl,magic-packet;
> + status = "okay";
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + clock-frequency = <5000000>;
> +
> + ethphy2: ethernet-phy@2 {
> + reg = <2>;
> + reset-assert-us = <10000>;
> + reset-deassert-us = <80000>;
> + reset-gpios = <&pcal6524 16 GPIO_ACTIVE_LOW>;
> + realtek,clkout-disable;
> + };
> + };
> +};
> +
> +&flexcan2 {
> + phys = <&flexcan2_phy>;
> + pinctrl-0 = <&pinctrl_flexcan2>;
> + pinctrl-1 = <&pinctrl_flexcan2_sleep>;
> + pinctrl-names = "default", "sleep";
> + status = "okay";
> +};
> +
> +&lpi2c1 {
> + clock-frequency = <400000>;
> + pinctrl-0 = <&pinctrl_lpi2c1>;
> + pinctrl-names = "default";
> + status = "okay";
> +
> + pcal6408: gpio@20 {
> + compatible = "nxp,pcal6408";
> + reg = <0x20>;
> + #gpio-cells = <2>;
> + gpio-controller;
> + reset-gpios = <&pcal6524 20 GPIO_ACTIVE_LOW>;
> + };
> +};
> +
> +&lpi2c2 {
> + clock-frequency = <400000>;
> + pinctrl-0 = <&pinctrl_lpi2c2>;
> + pinctrl-names = "default";
> + status = "okay";
> +
> + pcal6524: gpio@22 {
> + compatible = "nxp,pcal6524";
> + reg = <0x22>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + interrupt-parent = <&gpio3>;
> + interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> + #gpio-cells = <2>;
> + gpio-controller;
> + pinctrl-0 = <&pinctrl_pcal6524>;
> + pinctrl-names = "default";
> + /* does not boot with supplier set, because it is the bucks interrupt parent */
> + /* vcc-supply = <&buck4>; */
> + };
> +
> + pmic@25 {
> + compatible = "nxp,pca9451a";
> + reg = <0x25>;
> + interrupt-parent = <&pcal6524>;
> + interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
> +
> + regulators {
> +
> + buck1: BUCK1 {
> + regulator-name = "VDD_SOC_0V8";
> + regulator-min-microvolt = <610000>;
> + regulator-max-microvolt = <950000>;
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-ramp-delay = <3125>;
> + };
> +
> + buck2: BUCK2 {
> + regulator-name = "LPD4_x_VDDQ_0V6";
> + regulator-min-microvolt = <600000>;
> + regulator-max-microvolt = <670000>;
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-ramp-delay = <3125>;
> + };
> +
> + buck4: BUCK4 {
> + regulator-name = "VDD_3V3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + buck5: BUCK5 {
> + regulator-name = "VDD_1V8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + buck6: BUCK6 {
> + regulator-name = "LPD4_x_VDD2_1V1";
> + regulator-min-microvolt = <1060000>;
> + regulator-max-microvolt = <1140000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + ldo1: LDO1 {
> + regulator-name = "NVCC_BBSM_1V8";
> + regulator-min-microvolt = <1620000>;
> + regulator-max-microvolt = <1980000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + ldo4: LDO4 {
> + regulator-name = "VDD_ANA_0V8";
> + regulator-min-microvolt = <800000>;
> + regulator-max-microvolt = <840000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + ldo5: LDO5 {
> + regulator-name = "NVCC_SD";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> + };
> + };
> +
> + eeprom: eeprom@50 {
> + compatible = "atmel,24c256";
> + reg = <0x50>;
> + pagesize = <64>;
> + vcc-supply = <&buck4>;
> + };
> +};
> +
> +&lpi2c3 {
> + clock-frequency = <400000>;
> + pinctrl-0 = <&pinctrl_lpi2c3>;
> + pinctrl-names = "default";
> + status = "okay";
> +
> + ptn5110: tcpc@50 {
> + compatible = "nxp,ptn5110", "tcpci";
> + reg = <0x50>;
> + interrupt-parent = <&gpio3>;
> + interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> +
> + typec1_con: connector {
> + compatible = "usb-c-connector";
> + data-role = "dual";
> + label = "USB-C";
> + op-sink-microwatt = <15000000>;
> + power-role = "dual";
> + self-powered;
> + sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> + PDO_VAR(5000, 20000, 3000)>;
> + source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> + try-power-role = "sink";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + typec1_dr_sw: endpoint {
> + remote-endpoint = <&usb1_drd_sw>;
> + };
> + };
> + };
> + };
> + };
> +
> + pcf2131: rtc@53 {
> + compatible = "nxp,pcf2131";
> + reg = <0x53>;
> + interrupt-parent = <&pcal6524>;
> + interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
> + };
> +};
> +
> +&lpuart1 { /* console */
> + pinctrl-0 = <&pinctrl_uart1>;
> + pinctrl-names = "default";
> + status = "okay";
> +};
> +
> +&lpuart5 {
> + pinctrl-0 = <&pinctrl_uart5>;
> + pinctrl-names = "default";
> + status = "okay";
> +
> + uart-has-rtscts;
> +
> + bluetooth {
> + compatible = "nxp,88w8987-bt";
> + device-wakeup-gpios = <&pcal6408 3 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&pcal6524 19 GPIO_ACTIVE_LOW>;
> + vcc-supply = <®_usdhc3_vmmc>;
> + };
> +};
> +
> +&usbotg1 {
> + adp-disable;
> + disable-over-current;
> + dr_mode = "otg";
> + hnp-disable;
> + srp-disable;
> + usb-role-switch;
> + samsung,picophy-dc-vol-level-adjust = <7>;
> + samsung,picophy-pre-emp-curr-control = <3>;
> + status = "okay";
> +
> + port {
> + usb1_drd_sw: endpoint {
> + remote-endpoint = <&typec1_dr_sw>;
> + };
> + };
> +};
> +
> +&usbotg2 {
> + disable-over-current;
> + dr_mode = "host";
> + samsung,picophy-dc-vol-level-adjust = <7>;
> + samsung,picophy-pre-emp-curr-control = <3>;
> + status = "okay";
> +};
> +
> +&usdhc1 {
> + bus-width = <8>;
> + non-removable;
> + pinctrl-0 = <&pinctrl_usdhc1>;
> + pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
> + pinctrl-2 = <&pinctrl_usdhc1_200mhz>;
> + pinctrl-names = "default", "state_100mhz", "state_200mhz";
> + vmmc-supply = <&buck4>;
> + status = "okay";
> +};
> +
> +&usdhc2 {
> + bus-width = <4>;
> + cd-gpios = <&gpio3 00 GPIO_ACTIVE_LOW>;
> + no-mmc;
> + no-sdio;
> + pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> + pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
> + pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
> + pinctrl-3 = <&pinctrl_usdhc2_sleep>, <&pinctrl_usdhc2_gpio_sleep>;
> + pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep";
> + vmmc-supply = <®_usdhc2_vmmc>;
> + status = "okay";
> +};
> +
> +&wdog3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_wdog>;
> + fsl,ext-reset-output;
> + status = "okay";
> +};
> +
> +&iomuxc {
> +
> + pinctrl_eqos: eqosgrp {
> + fsl,pins = <
> + MX93_PAD_ENET1_MDC__ENET_QOS_MDC 0x57e
> + MX93_PAD_ENET1_MDIO__ENET_QOS_MDIO 0x57e
> + MX93_PAD_ENET1_RD0__ENET_QOS_RGMII_RD0 0x57e
> + MX93_PAD_ENET1_RD1__ENET_QOS_RGMII_RD1 0x57e
> + MX93_PAD_ENET1_RD2__ENET_QOS_RGMII_RD2 0x57e
> + MX93_PAD_ENET1_RD3__ENET_QOS_RGMII_RD3 0x57e
> + MX93_PAD_ENET1_RXC__CCM_ENET_QOS_CLOCK_GENERATE_RX_CLK 0x58e
> + MX93_PAD_ENET1_RX_CTL__ENET_QOS_RGMII_RX_CTL 0x57e
> + MX93_PAD_ENET1_TD0__ENET_QOS_RGMII_TD0 0x57e
> + MX93_PAD_ENET1_TD1__ENET_QOS_RGMII_TD1 0x57e
> + MX93_PAD_ENET1_TD2__ENET_QOS_RGMII_TD2 0x57e
> + MX93_PAD_ENET1_TD3__ENET_QOS_RGMII_TD3 0x57e
> + MX93_PAD_ENET1_TXC__CCM_ENET_QOS_CLOCK_GENERATE_TX_CLK 0x58e
> + MX93_PAD_ENET1_TX_CTL__ENET_QOS_RGMII_TX_CTL 0x57e
> + >;
> + };
> +
> + pinctrl_eqos_sleep: eqossleepgrp {
> + fsl,pins = <
> + MX93_PAD_ENET1_MDC__GPIO4_IO00 0x31e
> + MX93_PAD_ENET1_MDIO__GPIO4_IO01 0x31e
> + MX93_PAD_ENET1_RD0__GPIO4_IO10 0x31e
> + MX93_PAD_ENET1_RD1__GPIO4_IO11 0x31e
> + MX93_PAD_ENET1_RD2__GPIO4_IO12 0x31e
> + MX93_PAD_ENET1_RD3__GPIO4_IO13 0x31e
> + MX93_PAD_ENET1_RXC__GPIO4_IO09 0x31e
> + MX93_PAD_ENET1_RX_CTL__GPIO4_IO08 0x31e
> + MX93_PAD_ENET1_TD0__GPIO4_IO05 0x31e
> + MX93_PAD_ENET1_TD1__GPIO4_IO04 0x31e
> + MX93_PAD_ENET1_TD2__GPIO4_IO03 0x31e
> + MX93_PAD_ENET1_TD3__GPIO4_IO02 0x31e
> + MX93_PAD_ENET1_TXC__GPIO4_IO07 0x31e
> + MX93_PAD_ENET1_TX_CTL__GPIO4_IO06 0x31e
> + >;
> + };
> +
> + pinctrl_fec: fecgrp {
> + fsl,pins = <
> + MX93_PAD_ENET2_MDC__ENET1_MDC 0x57e
> + MX93_PAD_ENET2_MDIO__ENET1_MDIO 0x57e
> + MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0 0x57e
> + MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1 0x57e
> + MX93_PAD_ENET2_RD2__ENET1_RGMII_RD2 0x57e
> + MX93_PAD_ENET2_RD3__ENET1_RGMII_RD3 0x57e
> + MX93_PAD_ENET2_RXC__ENET1_RGMII_RXC 0x58e
> + MX93_PAD_ENET2_RX_CTL__ENET1_RGMII_RX_CTL 0x57e
> + MX93_PAD_ENET2_TD0__ENET1_RGMII_TD0 0x57e
> + MX93_PAD_ENET2_TD1__ENET1_RGMII_TD1 0x57e
> + MX93_PAD_ENET2_TD2__ENET1_RGMII_TD2 0x57e
> + MX93_PAD_ENET2_TD3__ENET1_RGMII_TD3 0x57e
> + MX93_PAD_ENET2_TXC__ENET1_RGMII_TXC 0x58e
> + MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL 0x57e
> + >;
> + };
> +
> + pinctrl_fec_sleep: fecsleepgrp {
> + fsl,pins = <
> + MX93_PAD_ENET2_MDC__GPIO4_IO14 0x51e
> + MX93_PAD_ENET2_MDIO__GPIO4_IO15 0x51e
> + MX93_PAD_ENET2_RD0__GPIO4_IO24 0x51e
> + MX93_PAD_ENET2_RD1__GPIO4_IO25 0x51e
> + MX93_PAD_ENET2_RD2__GPIO4_IO26 0x51e
> + MX93_PAD_ENET2_RD3__GPIO4_IO27 0x51e
> + MX93_PAD_ENET2_RXC__GPIO4_IO23 0x51e
> + MX93_PAD_ENET2_RX_CTL__GPIO4_IO22 0x51e
> + MX93_PAD_ENET2_TD0__GPIO4_IO19 0x51e
> + MX93_PAD_ENET2_TD1__GPIO4_IO18 0x51e
> + MX93_PAD_ENET2_TD2__GPIO4_IO17 0x51e
> + MX93_PAD_ENET2_TD3__GPIO4_IO16 0x51e
> + MX93_PAD_ENET2_TXC__GPIO4_IO21 0x51e
> + MX93_PAD_ENET2_TX_CTL__GPIO4_IO20 0x51e
> + >;
> + };
> +
> + pinctrl_flexcan2: flexcan2grp {
> + fsl,pins = <
> + MX93_PAD_GPIO_IO25__CAN2_TX 0x139e
> + MX93_PAD_GPIO_IO27__CAN2_RX 0x139e
> + >;
> + };
> +
> + pinctrl_flexcan2_sleep: flexcan2sleepgrp {
> + fsl,pins = <
> + MX93_PAD_GPIO_IO25__GPIO2_IO25 0x31e
> + MX93_PAD_GPIO_IO27__GPIO2_IO27 0x31e
> + >;
> + };
> +
> + pinctrl_lpi2c1: lpi2c1grp {
> + fsl,pins = <
> + MX93_PAD_I2C1_SCL__LPI2C1_SCL 0x40000b9e
> + MX93_PAD_I2C1_SDA__LPI2C1_SDA 0x40000b9e
> + >;
> + };
> +
> + pinctrl_lpi2c2: lpi2c2grp {
> + fsl,pins = <
> + MX93_PAD_I2C2_SCL__LPI2C2_SCL 0x40000b9e
> + MX93_PAD_I2C2_SDA__LPI2C2_SDA 0x40000b9e
> + >;
> + };
> +
> + pinctrl_lpi2c3: lpi2c3grp {
> + fsl,pins = <
> + MX93_PAD_GPIO_IO28__LPI2C3_SDA 0x40000b9e
> + MX93_PAD_GPIO_IO29__LPI2C3_SCL 0x40000b9e
> + >;
> + };
> +
> + pinctrl_pcal6524: pcal6524grp {
> + fsl,pins = <
> + MX93_PAD_CCM_CLKO2__GPIO3_IO27 0x31e
> + >;
> + };
> +
> + pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
> + fsl,pins = <
> + MX93_PAD_SD2_RESET_B__GPIO3_IO07 0x31e
> + >;
> + };
> +
> + pinctrl_uart1: uart1grp {
> + fsl,pins = <
> + MX93_PAD_UART1_RXD__LPUART1_RX 0x31e
> + MX93_PAD_UART1_TXD__LPUART1_TX 0x31e
> + >;
> + };
> +
> + pinctrl_uart5: uart5grp {
> + fsl,pins = <
> + MX93_PAD_DAP_TDO_TRACESWO__LPUART5_TX 0x31e
> + MX93_PAD_DAP_TDI__LPUART5_RX 0x31e
> + MX93_PAD_DAP_TMS_SWDIO__LPUART5_RTS_B 0x31e
> + MX93_PAD_DAP_TCLK_SWCLK__LPUART5_CTS_B 0x31e
> + >;
> + };
> +
> + /* need to config the SION for data and cmd pad, refer to ERR052021 */
> + pinctrl_usdhc1: usdhc1grp {
> + fsl,pins = <
> + MX93_PAD_SD1_CLK__USDHC1_CLK 0x1582
> + MX93_PAD_SD1_CMD__USDHC1_CMD 0x40001382
> + MX93_PAD_SD1_DATA0__USDHC1_DATA0 0x40001382
> + MX93_PAD_SD1_DATA1__USDHC1_DATA1 0x40001382
> + MX93_PAD_SD1_DATA2__USDHC1_DATA2 0x40001382
> + MX93_PAD_SD1_DATA3__USDHC1_DATA3 0x40001382
> + MX93_PAD_SD1_DATA4__USDHC1_DATA4 0x40001382
> + MX93_PAD_SD1_DATA5__USDHC1_DATA5 0x40001382
> + MX93_PAD_SD1_DATA6__USDHC1_DATA6 0x40001382
> + MX93_PAD_SD1_DATA7__USDHC1_DATA7 0x40001382
> + MX93_PAD_SD1_STROBE__USDHC1_STROBE 0x1582
> + >;
> + };
> +
> + /* need to config the SION for data and cmd pad, refer to ERR052021 */
> + pinctrl_usdhc1_100mhz: usdhc1-100mhzgrp {
> + fsl,pins = <
> + MX93_PAD_SD1_CLK__USDHC1_CLK 0x158e
> + MX93_PAD_SD1_CMD__USDHC1_CMD 0x4000138e
> + MX93_PAD_SD1_DATA0__USDHC1_DATA0 0x4000138e
> + MX93_PAD_SD1_DATA1__USDHC1_DATA1 0x4000138e
> + MX93_PAD_SD1_DATA2__USDHC1_DATA2 0x4000138e
> + MX93_PAD_SD1_DATA3__USDHC1_DATA3 0x4000138e
> + MX93_PAD_SD1_DATA4__USDHC1_DATA4 0x4000138e
> + MX93_PAD_SD1_DATA5__USDHC1_DATA5 0x4000138e
> + MX93_PAD_SD1_DATA6__USDHC1_DATA6 0x4000138e
> + MX93_PAD_SD1_DATA7__USDHC1_DATA7 0x4000138e
> + MX93_PAD_SD1_STROBE__USDHC1_STROBE 0x158e
> + >;
> + };
> +
> + /* need to config the SION for data and cmd pad, refer to ERR052021 */
> + pinctrl_usdhc1_200mhz: usdhc1-200mhzgrp {
> + fsl,pins = <
> + MX93_PAD_SD1_CLK__USDHC1_CLK 0x15fe
> + MX93_PAD_SD1_CMD__USDHC1_CMD 0x400013fe
> + MX93_PAD_SD1_DATA0__USDHC1_DATA0 0x400013fe
> + MX93_PAD_SD1_DATA1__USDHC1_DATA1 0x400013fe
> + MX93_PAD_SD1_DATA2__USDHC1_DATA2 0x400013fe
> + MX93_PAD_SD1_DATA3__USDHC1_DATA3 0x400013fe
> + MX93_PAD_SD1_DATA4__USDHC1_DATA4 0x400013fe
> + MX93_PAD_SD1_DATA5__USDHC1_DATA5 0x400013fe
> + MX93_PAD_SD1_DATA6__USDHC1_DATA6 0x400013fe
> + MX93_PAD_SD1_DATA7__USDHC1_DATA7 0x400013fe
> + MX93_PAD_SD1_STROBE__USDHC1_STROBE 0x15fe
> + >;
> + };
> +
> + pinctrl_usdhc2_gpio: usdhc2gpiogrp {
> + fsl,pins = <
> + MX93_PAD_SD2_CD_B__GPIO3_IO00 0x31e
> + >;
> + };
> +
> + pinctrl_usdhc2_gpio_sleep: usdhc2gpiosleepgrp {
> + fsl,pins = <
> + MX93_PAD_SD2_CD_B__GPIO3_IO00 0x51e
> + >;
> + };
> +
> + /* need to config the SION for data and cmd pad, refer to ERR052021 */
> + pinctrl_usdhc2: usdhc2grp {
> + fsl,pins = <
> + MX93_PAD_SD2_CLK__USDHC2_CLK 0x1582
> + MX93_PAD_SD2_CMD__USDHC2_CMD 0x40001382
> + MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x40001382
> + MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x40001382
> + MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x40001382
> + MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x40001382
> + MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> + >;
> + };
> +
> + /* need to config the SION for data and cmd pad, refer to ERR052021 */
> + pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> + fsl,pins = <
> + MX93_PAD_SD2_CLK__USDHC2_CLK 0x158e
> + MX93_PAD_SD2_CMD__USDHC2_CMD 0x4000138e
> + MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x4000138e
> + MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x4000138e
> + MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x4000138e
> + MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x4000138e
> + MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> + >;
> + };
> +
> + /* need to config the SION for data and cmd pad, refer to ERR052021 */
> + pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> + fsl,pins = <
> + MX93_PAD_SD2_CLK__USDHC2_CLK 0x15fe
> + MX93_PAD_SD2_CMD__USDHC2_CMD 0x400013fe
> + MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x400013fe
> + MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x400013fe
> + MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x400013fe
> + MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x400013fe
> + MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> + >;
> + };
> +
> + pinctrl_usdhc2_sleep: usdhc2-sleepgrp {
> + fsl,pins = <
> + MX93_PAD_SD2_CLK__GPIO3_IO01 0x51e
> + MX93_PAD_SD2_CMD__GPIO3_IO02 0x51e
> + MX93_PAD_SD2_DATA0__GPIO3_IO03 0x51e
> + MX93_PAD_SD2_DATA1__GPIO3_IO04 0x51e
> + MX93_PAD_SD2_DATA2__GPIO3_IO05 0x51e
> + MX93_PAD_SD2_DATA3__GPIO3_IO06 0x51e
> + MX93_PAD_SD2_VSELECT__GPIO3_IO19 0x51e
> + >;
> + };
> +
> + pinctrl_wdog: wdoggrp {
> + fsl,pins = <
> + MX93_PAD_WDOG_ANY__WDOG1_WDOG_ANY 0x31e
> + >;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
> index bd14ba28690c081817111aaabef12fb56a7c56a4..f2a31a0eacabff07532aee989eebd75a0c3e22e0 100644
> --- a/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
> +++ b/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
> @@ -1,71 +1,14 @@
> // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> /dts-v1/;
>
> -#include <dt-bindings/usb/pd.h>
> #include "imx93.dtsi"
> +#include "imx93-11x11-frdm-common.dtsi"
>
> / {
> compatible = "fsl,imx93-11x11-frdm", "fsl,imx93";
> model = "NXP i.MX93 11X11 FRDM board";
>
> - aliases {
> - can0 = &flexcan2;
> - ethernet0 = &fec;
> - ethernet1 = &eqos;
> - i2c0 = &lpi2c1;
> - i2c1 = &lpi2c2;
> - i2c2 = &lpi2c3;
> - mmc0 = &usdhc1; /* EMMC */
> - mmc1 = &usdhc2; /* uSD */
> - rtc0 = &pcf2131;
> - serial0 = &lpuart1;
> - serial4 = &lpuart5;
> - };
> -
> - chosen {
> - stdout-path = &lpuart1;
> - };
> -
> - flexcan2_phy: can-phy {
> - compatible = "nxp,tja1051";
> - #phy-cells = <0>;
> - max-bitrate = <5000000>;
> - silent-gpios = <&pcal6524 23 GPIO_ACTIVE_HIGH>;
> - };
> -
> - gpio-keys {
> - compatible = "gpio-keys";
> -
> - button-k2 {
> - label = "Button K2";
> - linux,code = <BTN_1>;
> - gpios = <&pcal6524 5 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> - interrupt-parent = <&pcal6524>;
> - interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
> - };
> -
> - button-k3 {
> - label = "Button K3";
> - linux,code = <BTN_2>;
> - gpios = <&pcal6524 6 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> - interrupt-parent = <&pcal6524>;
> - interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
> - };
> - };
> -
> - reg_usdhc2_vmmc: regulator-usdhc2 {
> - compatible = "regulator-fixed";
> - off-on-delay-us = <12000>;
> - pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> - pinctrl-names = "default";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-name = "VSD_3V3";
> - vin-supply = <&buck4>;
> - gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
> - enable-active-high;
> - };
> -
> reg_usdhc3_vmmc: regulator-usdhc3 {
> compatible = "regulator-fixed";
> regulator-name = "VPCIe_3V3";
> @@ -76,51 +19,6 @@ reg_usdhc3_vmmc: regulator-usdhc3 {
> enable-active-high;
> };
>
> - reserved-memory {
> - ranges;
> - #address-cells = <2>;
> - #size-cells = <2>;
> -
> - linux,cma {
> - compatible = "shared-dma-pool";
> - alloc-ranges = <0 0x80000000 0 0x30000000>;
> - reusable;
> - size = <0 0x10000000>;
> - linux,cma-default;
> - };
> -
> - rsc_table: rsc-table@2021e000 {
> - reg = <0 0x2021e000 0 0x1000>;
> - no-map;
> - };
> -
> - vdev0vring0: vdev0vring0@a4000000 {
> - reg = <0 0xa4000000 0 0x8000>;
> - no-map;
> - };
> -
> - vdev0vring1: vdev0vring1@a4008000 {
> - reg = <0 0xa4008000 0 0x8000>;
> - no-map;
> - };
> -
> - vdev1vring0: vdev1vring0@a4010000 {
> - reg = <0 0xa4010000 0 0x8000>;
> - no-map;
> - };
> -
> - vdev1vring1: vdev1vring1@a4018000 {
> - reg = <0 0xa4018000 0 0x8000>;
> - no-map;
> - };
> -
> - vdevbuffer: vdevbuffer@a4020000 {
> - compatible = "shared-dma-pool";
> - reg = <0 0xa4020000 0 0x100000>;
> - no-map;
> - };
> - };
> -
> sound-mqs {
> compatible = "fsl,imx-audio-mqs";
> model = "mqs-audio";
> @@ -134,269 +32,6 @@ usdhc3_pwrseq: mmc-pwrseq {
> };
> };
>
> -&adc1 {
> - vref-supply = <&buck5>;
> - status = "okay";
> -};
> -
> -&mu1 {
> - status = "okay";
> -};
> -
> -&cm33 {
> - mboxes = <&mu1 0 1>,
> - <&mu1 1 1>,
> - <&mu1 3 1>;
> - mbox-names = "tx", "rx", "rxdb";
> - memory-region = <&vdevbuffer>, <&vdev0vring0>, <&vdev0vring1>,
> - <&vdev1vring0>, <&vdev1vring1>, <&rsc_table>;
> - status = "okay";
> -};
> -
> -&eqos {
> - pinctrl-names = "default", "sleep";
> - pinctrl-0 = <&pinctrl_eqos>;
> - pinctrl-1 = <&pinctrl_eqos_sleep>;
> - phy-handle = <ðphy1>;
> - phy-mode = "rgmii-id";
> - status = "okay";
> -
> - mdio {
> - compatible = "snps,dwmac-mdio";
> - #address-cells = <1>;
> - #size-cells = <0>;
> - clock-frequency = <5000000>;
> -
> - ethphy1: ethernet-phy@1 {
> - reg = <1>;
> - reset-assert-us = <10000>;
> - reset-deassert-us = <80000>;
> - reset-gpios = <&pcal6524 15 GPIO_ACTIVE_LOW>;
> - realtek,clkout-disable;
> - };
> - };
> -};
> -
> -&fec {
> - pinctrl-names = "default", "sleep";
> - pinctrl-0 = <&pinctrl_fec>;
> - pinctrl-1 = <&pinctrl_fec_sleep>;
> - phy-mode = "rgmii-id";
> - phy-handle = <ðphy2>;
> - fsl,magic-packet;
> - status = "okay";
> -
> - mdio {
> - #address-cells = <1>;
> - #size-cells = <0>;
> - clock-frequency = <5000000>;
> -
> - ethphy2: ethernet-phy@2 {
> - reg = <2>;
> - reset-assert-us = <10000>;
> - reset-deassert-us = <80000>;
> - reset-gpios = <&pcal6524 16 GPIO_ACTIVE_LOW>;
> - realtek,clkout-disable;
> - };
> - };
> -};
> -
> -&flexcan2 {
> - phys = <&flexcan2_phy>;
> - pinctrl-0 = <&pinctrl_flexcan2>;
> - pinctrl-1 = <&pinctrl_flexcan2_sleep>;
> - pinctrl-names = "default", "sleep";
> - status = "okay";
> -};
> -
> -&lpi2c1 {
> - clock-frequency = <400000>;
> - pinctrl-0 = <&pinctrl_lpi2c1>;
> - pinctrl-names = "default";
> - status = "okay";
> -
> - pcal6408: gpio@20 {
> - compatible = "nxp,pcal6408";
> - reg = <0x20>;
> - #gpio-cells = <2>;
> - gpio-controller;
> - reset-gpios = <&pcal6524 20 GPIO_ACTIVE_LOW>;
> - };
> -};
> -
> -&lpi2c2 {
> - clock-frequency = <400000>;
> - pinctrl-0 = <&pinctrl_lpi2c2>;
> - pinctrl-names = "default";
> - status = "okay";
> -
> - pcal6524: gpio@22 {
> - compatible = "nxp,pcal6524";
> - reg = <0x22>;
> - #interrupt-cells = <2>;
> - interrupt-controller;
> - interrupt-parent = <&gpio3>;
> - interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> - #gpio-cells = <2>;
> - gpio-controller;
> - pinctrl-0 = <&pinctrl_pcal6524>;
> - pinctrl-names = "default";
> - /* does not boot with supplier set, because it is the bucks interrupt parent */
> - /* vcc-supply = <&buck4>; */
> - };
> -
> - pmic@25 {
> - compatible = "nxp,pca9451a";
> - reg = <0x25>;
> - interrupt-parent = <&pcal6524>;
> - interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
> -
> - regulators {
> -
> - buck1: BUCK1 {
> - regulator-name = "VDD_SOC_0V8";
> - regulator-min-microvolt = <610000>;
> - regulator-max-microvolt = <950000>;
> - regulator-always-on;
> - regulator-boot-on;
> - regulator-ramp-delay = <3125>;
> - };
> -
> - buck2: BUCK2 {
> - regulator-name = "LPD4_x_VDDQ_0V6";
> - regulator-min-microvolt = <600000>;
> - regulator-max-microvolt = <670000>;
> - regulator-always-on;
> - regulator-boot-on;
> - regulator-ramp-delay = <3125>;
> - };
> -
> - buck4: BUCK4 {
> - regulator-name = "VDD_3V3";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> -
> - buck5: BUCK5 {
> - regulator-name = "VDD_1V8";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> -
> - buck6: BUCK6 {
> - regulator-name = "LPD4_x_VDD2_1V1";
> - regulator-min-microvolt = <1060000>;
> - regulator-max-microvolt = <1140000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> -
> - ldo1: LDO1 {
> - regulator-name = "NVCC_BBSM_1V8";
> - regulator-min-microvolt = <1620000>;
> - regulator-max-microvolt = <1980000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> -
> - ldo4: LDO4 {
> - regulator-name = "VDD_ANA_0V8";
> - regulator-min-microvolt = <800000>;
> - regulator-max-microvolt = <840000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> -
> - ldo5: LDO5 {
> - regulator-name = "NVCC_SD";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> - };
> - };
> -
> - eeprom: eeprom@50 {
> - compatible = "atmel,24c256";
> - reg = <0x50>;
> - pagesize = <64>;
> - vcc-supply = <&buck4>;
> - };
> -};
> -
> -&lpi2c3 {
> - clock-frequency = <400000>;
> - pinctrl-0 = <&pinctrl_lpi2c3>;
> - pinctrl-names = "default";
> - status = "okay";
> -
> - ptn5110: tcpc@50 {
> - compatible = "nxp,ptn5110", "tcpci";
> - reg = <0x50>;
> - interrupt-parent = <&gpio3>;
> - interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> -
> - typec1_con: connector {
> - compatible = "usb-c-connector";
> - data-role = "dual";
> - label = "USB-C";
> - op-sink-microwatt = <15000000>;
> - power-role = "dual";
> - self-powered;
> - sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> - PDO_VAR(5000, 20000, 3000)>;
> - source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> - try-power-role = "sink";
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - port@0 {
> - reg = <0>;
> -
> - typec1_dr_sw: endpoint {
> - remote-endpoint = <&usb1_drd_sw>;
> - };
> - };
> - };
> - };
> - };
> -
> - pcf2131: rtc@53 {
> - compatible = "nxp,pcf2131";
> - reg = <0x53>;
> - interrupt-parent = <&pcal6524>;
> - interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
> - };
> -};
> -
> -&lpuart1 { /* console */
> - pinctrl-0 = <&pinctrl_uart1>;
> - pinctrl-names = "default";
> - status = "okay";
> -};
> -
> -&lpuart5 {
> - pinctrl-0 = <&pinctrl_uart5>;
> - pinctrl-names = "default";
> - status = "okay";
> -
> - uart-has-rtscts;
> -
> - bluetooth {
> - compatible = "nxp,88w8987-bt";
> - device-wakeup-gpios = <&pcal6408 3 GPIO_ACTIVE_HIGH>;
> - reset-gpios = <&pcal6524 19 GPIO_ACTIVE_LOW>;
> - vcc-supply = <®_usdhc3_vmmc>;
> - };
> -};
> -
> &mqs1 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_mqs1>;
> @@ -418,57 +53,6 @@ &sai1 {
> status = "okay";
> };
>
> -&usbotg1 {
> - adp-disable;
> - disable-over-current;
> - dr_mode = "otg";
> - hnp-disable;
> - srp-disable;
> - usb-role-switch;
> - samsung,picophy-dc-vol-level-adjust = <7>;
> - samsung,picophy-pre-emp-curr-control = <3>;
> - status = "okay";
> -
> - port {
> - usb1_drd_sw: endpoint {
> - remote-endpoint = <&typec1_dr_sw>;
> - };
> - };
> -};
> -
> -&usbotg2 {
> - disable-over-current;
> - dr_mode = "host";
> - samsung,picophy-dc-vol-level-adjust = <7>;
> - samsung,picophy-pre-emp-curr-control = <3>;
> - status = "okay";
> -};
> -
> -&usdhc1 {
> - bus-width = <8>;
> - non-removable;
> - pinctrl-0 = <&pinctrl_usdhc1>;
> - pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
> - pinctrl-2 = <&pinctrl_usdhc1_200mhz>;
> - pinctrl-names = "default", "state_100mhz", "state_200mhz";
> - vmmc-supply = <&buck4>;
> - status = "okay";
> -};
> -
> -&usdhc2 {
> - bus-width = <4>;
> - cd-gpios = <&gpio3 00 GPIO_ACTIVE_LOW>;
> - no-mmc;
> - no-sdio;
> - pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> - pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
> - pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
> - pinctrl-3 = <&pinctrl_usdhc2_sleep>, <&pinctrl_usdhc2_gpio_sleep>;
> - pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep";
> - vmmc-supply = <®_usdhc2_vmmc>;
> - status = "okay";
> -};
> -
> &usdhc3 {
> bus-width = <4>;
> keep-power-in-suspend;
> @@ -483,126 +67,8 @@ &usdhc3 {
> status = "okay";
> };
>
> -&wdog3 {
> - pinctrl-names = "default";
> - pinctrl-0 = <&pinctrl_wdog>;
> - fsl,ext-reset-output;
> - status = "okay";
> -};
>
> &iomuxc {
> -
> - pinctrl_eqos: eqosgrp {
> - fsl,pins = <
> - MX93_PAD_ENET1_MDC__ENET_QOS_MDC 0x57e
> - MX93_PAD_ENET1_MDIO__ENET_QOS_MDIO 0x57e
> - MX93_PAD_ENET1_RD0__ENET_QOS_RGMII_RD0 0x57e
> - MX93_PAD_ENET1_RD1__ENET_QOS_RGMII_RD1 0x57e
> - MX93_PAD_ENET1_RD2__ENET_QOS_RGMII_RD2 0x57e
> - MX93_PAD_ENET1_RD3__ENET_QOS_RGMII_RD3 0x57e
> - MX93_PAD_ENET1_RXC__CCM_ENET_QOS_CLOCK_GENERATE_RX_CLK 0x58e
> - MX93_PAD_ENET1_RX_CTL__ENET_QOS_RGMII_RX_CTL 0x57e
> - MX93_PAD_ENET1_TD0__ENET_QOS_RGMII_TD0 0x57e
> - MX93_PAD_ENET1_TD1__ENET_QOS_RGMII_TD1 0x57e
> - MX93_PAD_ENET1_TD2__ENET_QOS_RGMII_TD2 0x57e
> - MX93_PAD_ENET1_TD3__ENET_QOS_RGMII_TD3 0x57e
> - MX93_PAD_ENET1_TXC__CCM_ENET_QOS_CLOCK_GENERATE_TX_CLK 0x58e
> - MX93_PAD_ENET1_TX_CTL__ENET_QOS_RGMII_TX_CTL 0x57e
> - >;
> - };
> -
> - pinctrl_eqos_sleep: eqossleepgrp {
> - fsl,pins = <
> - MX93_PAD_ENET1_MDC__GPIO4_IO00 0x31e
> - MX93_PAD_ENET1_MDIO__GPIO4_IO01 0x31e
> - MX93_PAD_ENET1_RD0__GPIO4_IO10 0x31e
> - MX93_PAD_ENET1_RD1__GPIO4_IO11 0x31e
> - MX93_PAD_ENET1_RD2__GPIO4_IO12 0x31e
> - MX93_PAD_ENET1_RD3__GPIO4_IO13 0x31e
> - MX93_PAD_ENET1_RXC__GPIO4_IO09 0x31e
> - MX93_PAD_ENET1_RX_CTL__GPIO4_IO08 0x31e
> - MX93_PAD_ENET1_TD0__GPIO4_IO05 0x31e
> - MX93_PAD_ENET1_TD1__GPIO4_IO04 0x31e
> - MX93_PAD_ENET1_TD2__GPIO4_IO03 0x31e
> - MX93_PAD_ENET1_TD3__GPIO4_IO02 0x31e
> - MX93_PAD_ENET1_TXC__GPIO4_IO07 0x31e
> - MX93_PAD_ENET1_TX_CTL__GPIO4_IO06 0x31e
> - >;
> - };
> -
> - pinctrl_fec: fecgrp {
> - fsl,pins = <
> - MX93_PAD_ENET2_MDC__ENET1_MDC 0x57e
> - MX93_PAD_ENET2_MDIO__ENET1_MDIO 0x57e
> - MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0 0x57e
> - MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1 0x57e
> - MX93_PAD_ENET2_RD2__ENET1_RGMII_RD2 0x57e
> - MX93_PAD_ENET2_RD3__ENET1_RGMII_RD3 0x57e
> - MX93_PAD_ENET2_RXC__ENET1_RGMII_RXC 0x58e
> - MX93_PAD_ENET2_RX_CTL__ENET1_RGMII_RX_CTL 0x57e
> - MX93_PAD_ENET2_TD0__ENET1_RGMII_TD0 0x57e
> - MX93_PAD_ENET2_TD1__ENET1_RGMII_TD1 0x57e
> - MX93_PAD_ENET2_TD2__ENET1_RGMII_TD2 0x57e
> - MX93_PAD_ENET2_TD3__ENET1_RGMII_TD3 0x57e
> - MX93_PAD_ENET2_TXC__ENET1_RGMII_TXC 0x58e
> - MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL 0x57e
> - >;
> - };
> -
> - pinctrl_fec_sleep: fecsleepgrp {
> - fsl,pins = <
> - MX93_PAD_ENET2_MDC__GPIO4_IO14 0x51e
> - MX93_PAD_ENET2_MDIO__GPIO4_IO15 0x51e
> - MX93_PAD_ENET2_RD0__GPIO4_IO24 0x51e
> - MX93_PAD_ENET2_RD1__GPIO4_IO25 0x51e
> - MX93_PAD_ENET2_RD2__GPIO4_IO26 0x51e
> - MX93_PAD_ENET2_RD3__GPIO4_IO27 0x51e
> - MX93_PAD_ENET2_RXC__GPIO4_IO23 0x51e
> - MX93_PAD_ENET2_RX_CTL__GPIO4_IO22 0x51e
> - MX93_PAD_ENET2_TD0__GPIO4_IO19 0x51e
> - MX93_PAD_ENET2_TD1__GPIO4_IO18 0x51e
> - MX93_PAD_ENET2_TD2__GPIO4_IO17 0x51e
> - MX93_PAD_ENET2_TD3__GPIO4_IO16 0x51e
> - MX93_PAD_ENET2_TXC__GPIO4_IO21 0x51e
> - MX93_PAD_ENET2_TX_CTL__GPIO4_IO20 0x51e
> - >;
> - };
> -
> - pinctrl_flexcan2: flexcan2grp {
> - fsl,pins = <
> - MX93_PAD_GPIO_IO25__CAN2_TX 0x139e
> - MX93_PAD_GPIO_IO27__CAN2_RX 0x139e
> - >;
> - };
> -
> - pinctrl_flexcan2_sleep: flexcan2sleepgrp {
> - fsl,pins = <
> - MX93_PAD_GPIO_IO25__GPIO2_IO25 0x31e
> - MX93_PAD_GPIO_IO27__GPIO2_IO27 0x31e
> - >;
> - };
> -
> - pinctrl_lpi2c1: lpi2c1grp {
> - fsl,pins = <
> - MX93_PAD_I2C1_SCL__LPI2C1_SCL 0x40000b9e
> - MX93_PAD_I2C1_SDA__LPI2C1_SDA 0x40000b9e
> - >;
> - };
> -
> - pinctrl_lpi2c2: lpi2c2grp {
> - fsl,pins = <
> - MX93_PAD_I2C2_SCL__LPI2C2_SCL 0x40000b9e
> - MX93_PAD_I2C2_SDA__LPI2C2_SDA 0x40000b9e
> - >;
> - };
> -
> - pinctrl_lpi2c3: lpi2c3grp {
> - fsl,pins = <
> - MX93_PAD_GPIO_IO28__LPI2C3_SDA 0x40000b9e
> - MX93_PAD_GPIO_IO29__LPI2C3_SCL 0x40000b9e
> - >;
> - };
> -
> pinctrl_mqs1: mqs1grp {
> fsl,pins = <
> MX93_PAD_PDM_CLK__MQS1_LEFT 0x31e
> @@ -610,149 +76,7 @@ MX93_PAD_PDM_BIT_STREAM0__MQS1_RIGHT 0x31e
> >;
> };
>
> - pinctrl_pcal6524: pcal6524grp {
> - fsl,pins = <
> - MX93_PAD_CCM_CLKO2__GPIO3_IO27 0x31e
> - >;
> - };
> -
> - pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
> - fsl,pins = <
> - MX93_PAD_SD2_RESET_B__GPIO3_IO07 0x31e
> - >;
> - };
> -
> - pinctrl_uart1: uart1grp {
> - fsl,pins = <
> - MX93_PAD_UART1_RXD__LPUART1_RX 0x31e
> - MX93_PAD_UART1_TXD__LPUART1_TX 0x31e
> - >;
> - };
> -
> - pinctrl_uart5: uart5grp {
> - fsl,pins = <
> - MX93_PAD_DAP_TDO_TRACESWO__LPUART5_TX 0x31e
> - MX93_PAD_DAP_TDI__LPUART5_RX 0x31e
> - MX93_PAD_DAP_TMS_SWDIO__LPUART5_RTS_B 0x31e
> - MX93_PAD_DAP_TCLK_SWCLK__LPUART5_CTS_B 0x31e
> - >;
> - };
> -
> - /* need to config the SION for data and cmd pad, refer to ERR052021 */
> - pinctrl_usdhc1: usdhc1grp {
> - fsl,pins = <
> - MX93_PAD_SD1_CLK__USDHC1_CLK 0x1582
> - MX93_PAD_SD1_CMD__USDHC1_CMD 0x40001382
> - MX93_PAD_SD1_DATA0__USDHC1_DATA0 0x40001382
> - MX93_PAD_SD1_DATA1__USDHC1_DATA1 0x40001382
> - MX93_PAD_SD1_DATA2__USDHC1_DATA2 0x40001382
> - MX93_PAD_SD1_DATA3__USDHC1_DATA3 0x40001382
> - MX93_PAD_SD1_DATA4__USDHC1_DATA4 0x40001382
> - MX93_PAD_SD1_DATA5__USDHC1_DATA5 0x40001382
> - MX93_PAD_SD1_DATA6__USDHC1_DATA6 0x40001382
> - MX93_PAD_SD1_DATA7__USDHC1_DATA7 0x40001382
> - MX93_PAD_SD1_STROBE__USDHC1_STROBE 0x1582
> - >;
> - };
> -
> - /* need to config the SION for data and cmd pad, refer to ERR052021 */
> - pinctrl_usdhc1_100mhz: usdhc1-100mhzgrp {
> - fsl,pins = <
> - MX93_PAD_SD1_CLK__USDHC1_CLK 0x158e
> - MX93_PAD_SD1_CMD__USDHC1_CMD 0x4000138e
> - MX93_PAD_SD1_DATA0__USDHC1_DATA0 0x4000138e
> - MX93_PAD_SD1_DATA1__USDHC1_DATA1 0x4000138e
> - MX93_PAD_SD1_DATA2__USDHC1_DATA2 0x4000138e
> - MX93_PAD_SD1_DATA3__USDHC1_DATA3 0x4000138e
> - MX93_PAD_SD1_DATA4__USDHC1_DATA4 0x4000138e
> - MX93_PAD_SD1_DATA5__USDHC1_DATA5 0x4000138e
> - MX93_PAD_SD1_DATA6__USDHC1_DATA6 0x4000138e
> - MX93_PAD_SD1_DATA7__USDHC1_DATA7 0x4000138e
> - MX93_PAD_SD1_STROBE__USDHC1_STROBE 0x158e
> - >;
> - };
> -
> - /* need to config the SION for data and cmd pad, refer to ERR052021 */
> - pinctrl_usdhc1_200mhz: usdhc1-200mhzgrp {
> - fsl,pins = <
> - MX93_PAD_SD1_CLK__USDHC1_CLK 0x15fe
> - MX93_PAD_SD1_CMD__USDHC1_CMD 0x400013fe
> - MX93_PAD_SD1_DATA0__USDHC1_DATA0 0x400013fe
> - MX93_PAD_SD1_DATA1__USDHC1_DATA1 0x400013fe
> - MX93_PAD_SD1_DATA2__USDHC1_DATA2 0x400013fe
> - MX93_PAD_SD1_DATA3__USDHC1_DATA3 0x400013fe
> - MX93_PAD_SD1_DATA4__USDHC1_DATA4 0x400013fe
> - MX93_PAD_SD1_DATA5__USDHC1_DATA5 0x400013fe
> - MX93_PAD_SD1_DATA6__USDHC1_DATA6 0x400013fe
> - MX93_PAD_SD1_DATA7__USDHC1_DATA7 0x400013fe
> - MX93_PAD_SD1_STROBE__USDHC1_STROBE 0x15fe
> - >;
> - };
> -
> - pinctrl_usdhc2_gpio: usdhc2gpiogrp {
> - fsl,pins = <
> - MX93_PAD_SD2_CD_B__GPIO3_IO00 0x31e
> - >;
> - };
> -
> - pinctrl_usdhc2_gpio_sleep: usdhc2gpiosleepgrp {
> - fsl,pins = <
> - MX93_PAD_SD2_CD_B__GPIO3_IO00 0x51e
> - >;
> - };
> -
> - /* need to config the SION for data and cmd pad, refer to ERR052021 */
> - pinctrl_usdhc2: usdhc2grp {
> - fsl,pins = <
> - MX93_PAD_SD2_CLK__USDHC2_CLK 0x1582
> - MX93_PAD_SD2_CMD__USDHC2_CMD 0x40001382
> - MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x40001382
> - MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x40001382
> - MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x40001382
> - MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x40001382
> - MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> - >;
> - };
> -
> /* need to config the SION for data and cmd pad, refer to ERR052021 */
> - pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> - fsl,pins = <
> - MX93_PAD_SD2_CLK__USDHC2_CLK 0x158e
> - MX93_PAD_SD2_CMD__USDHC2_CMD 0x4000138e
> - MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x4000138e
> - MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x4000138e
> - MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x4000138e
> - MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x4000138e
> - MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> - >;
> - };
> -
> - /* need to config the SION for data and cmd pad, refer to ERR052021 */
> - pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> - fsl,pins = <
> - MX93_PAD_SD2_CLK__USDHC2_CLK 0x15fe
> - MX93_PAD_SD2_CMD__USDHC2_CMD 0x400013fe
> - MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x400013fe
> - MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x400013fe
> - MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x400013fe
> - MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x400013fe
> - MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> - >;
> - };
> -
> - pinctrl_usdhc2_sleep: usdhc2-sleepgrp {
> - fsl,pins = <
> - MX93_PAD_SD2_CLK__GPIO3_IO01 0x51e
> - MX93_PAD_SD2_CMD__GPIO3_IO02 0x51e
> - MX93_PAD_SD2_DATA0__GPIO3_IO03 0x51e
> - MX93_PAD_SD2_DATA1__GPIO3_IO04 0x51e
> - MX93_PAD_SD2_DATA2__GPIO3_IO05 0x51e
> - MX93_PAD_SD2_DATA3__GPIO3_IO06 0x51e
> - MX93_PAD_SD2_VSELECT__GPIO3_IO19 0x51e
> - >;
> - };
> -
> - /* need to config the SION for data and cmd pad, refer to ERR052021 */
> pinctrl_usdhc3: usdhc3grp {
> fsl,pins = <
> MX93_PAD_SD3_CLK__USDHC3_CLK 0x1582
> @@ -798,10 +122,4 @@ MX93_PAD_SD3_DATA2__GPIO3_IO24 0x31e
> MX93_PAD_SD3_DATA3__GPIO3_IO25 0x31e
> >;
> };
> -
> - pinctrl_wdog: wdoggrp {
> - fsl,pins = <
> - MX93_PAD_WDOG_ANY__WDOG1_WDOG_ANY 0x31e
> - >;
> - };
> };
>
> --
> 2.34.1
>
>
^ permalink raw reply
* Re: [PATCH v5 02/16] device property: Add fwnode_graph_get_next_port_endpoint()
From: Andy Shevchenko @ 2026-07-17 16:33 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: sashiko-reviews, devicetree, driver-core, Bartosz Golaszewski
In-Reply-To: <CAGXv+5HS3neb=8EW0FT2-tP-dhPFqQ+q8YRS8Gy_XBic4z4Mug@mail.gmail.com>
On Fri, Jul 17, 2026 at 07:05:56PM +0800, Chen-Yu Tsai wrote:
> On Fri, Jul 17, 2026 at 5:04 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Jul 16, 2026 at 04:52:27PM +0800, Chen-Yu Tsai wrote:
> > > On Wed, Jul 15, 2026 at 5:09 PM <sashiko-bot@kernel.org> wrote:
...
> > > > > +struct fwnode_handle *fwnode_graph_get_next_port_endpoint(const struct fwnode_handle *port,
> > > > > + struct fwnode_handle *prev)
> > > > > +{
> > > > > + do {
> > > > > + prev = fwnode_get_next_child_node(port, prev);
> > > >
> > > > [Severity: High]
> > > > This is a pre-existing issue, but iterating over endpoints using
> > > > fwnode_get_next_child_node() can cause an infinite loop and a use-after-free
> > > > if a device has secondary fwnodes attached.
> > > >
> > > > If a device port has both primary children and a secondary fwnode, the primary
> > > > ops finish and drop the reference to the last primary child, returning NULL.
> > > > Then, fwnode_get_next_child_node() incorrectly passes this same dropped child
> > > > pointer to the secondary ops:
> > > >
> > > > drivers/base/property.c:fwnode_get_next_child_node() {
> > > > /* Try to find a child in primary fwnode */
> > > > next = fwnode_call_ptr_op(fwnode, get_next_child_node, child);
> > > > if (next)
> > > > return next;
> > > >
> > > > /* When no more children in primary, continue with secondary */
> > > > return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
> > > > }
> > > >
> > > > This can result in a double-put. Furthermore, when traversing secondary
> > > > children, the primary ops don't recognize the software node, treat it as NULL,
> > > > and return the first primary child again, creating an infinite loop.
> >
> > Yeah, it has to be implemented in a way how fwnode_graph_get_next_endpoint() is done.
>
> If your patch series addresses the issue, I guess we can consider it
> covered? Sashiko labeled this a pre-existing.
I noticed that it addresses only main get_next case, but get_next_available is
still with the same issue. It's not that critical, we don't use much that API,
but still will be good to fix at some point.
> > > > [Severity: Medium]
> > > > Does this fail to unconditionally put the prev reference if the provided port
> > > > is invalid?
> > > >
> > > > If a caller passes a valid prev node but port is NULL or an error pointer,
> > > > fwnode_get_next_child_node() returns early without dropping prev:
> > > >
> > > > drivers/base/property.c:fwnode_get_next_child_node() {
> > > > struct fwnode_handle *next;
> > > >
> > > > if (IS_ERR_OR_NULL(fwnode))
> > > > return NULL;
> > > > ...
> > > > }
> > > >
> > > > Could this lead to a reference count leak, violating the documented API
> > > > contract that prev is unconditionally put?
> > >
> > > This seems like what the other fwnode_get_next_*() functions are doing.
> > > I'd say this is a pre-existing issue as well.
> >
> > See this series (not yet applied, you even have chance to review it!):
> > 20260611203537.1786399-1-andriy.shevchenko@linux.intel.com
>
> I believe that series addresses the first issue, not this one, even
> though it partially mitigates it.
>
> The contract says "puts a reference to @prev unconditionally." What it
> doesn't say is that it requires @node to be valid, i.e. the function
> is a no-op if node == NULL.
>
> "node != NULL" being an implicit requirement seems to be the norm for
> the fwnode_* and of_* APIs. It's not explicitly spelled out and Sashiko
> lacks the context for it.
>
> I'm not sure this should be changed within this patch. It would end up
> as a mismatch between the various functions and possibly lead to more
> confusion.
How does node become invalid after that patch?
> > > > > + if (fwnode_name_eq(prev, "endpoint"))
> > > >
> > > > [Severity: High]
> > > > Does hardcoding this string match break the fwnode graph abstraction for
> > > > non-DeviceTree platforms?
> > > >
> > > > By strictly matching the "endpoint" prefix, this bypasses the provider-specific
> > > > fwnode_operations, which could silently ignore valid ACPI and software node
> > > > endpoints that don't follow this exact naming scheme. Shouldn't this rely on
> > > > the backend-specific graph_get_next_endpoint operations instead?
> > >
> > > From drivers/acpi/property.c it seems that ACPI graphs follow the same
> > > structure. I don't have visibility into ACPI implementations though.
> >
> > Sashiko might be right. ACPI has device and data nodes, for device nodes the
> > name will be FourCC, so never longer than 4 characters. For data nodes, it
> > takes their names, which are arbitrary strings and seems should follow the given
> > schema. You need Sakari Ailus to review this patch.
>
> OK. Will add Sakari in the next version.
Hmm... This is interesting. For any fwnode API changes you should add the
respective reviewers. Do you use tools or doing that manually? You should use
tools. I know that `b4` is capable of doing that, but I use a script [1] I
wrote a few years ago.
> > > We also have the following in include/linux/fwnode.h:
> > >
> > > #define SWNODE_GRAPH_PORT_NAME_FMT "port@%u"
> > > #define SWNODE_GRAPH_ENDPOINT_NAME_FMT "endpoint@%u"
> > >
> > > So this should not be a problem.
> > >
> > > > > + break;
> > > > > + } while (prev);
> > > > > +
> > > > > + return prev;
> > > > > +}
[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 3/4] iio: adc: ltc2378: Enable high-speed data capture
From: Marcelo Schmitt @ 2026-07-17 16:35 UTC (permalink / raw)
To: sashiko-reviews
Cc: Marcelo Schmitt, robh, conor+dt, devicetree, linux-iio,
marcelo.schmitt1
In-Reply-To: <20260716212451.923951F000E9@smtp.kernel.org>
On 07/16, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Integer divide-by-zero leading to kernel panic in `ltc2378_update_conversion_rate` when a sampling frequency of 1 Hz is requested and the PWM driver rounds the period length above 1 second.
> --
>
> --- Patch [3]: [PATCH v7 3/4] iio: adc: ltc2378: Enable high-speed data capture ---
> commit ef54f2c79f2cac1649922032814adc7fcc5f9684
> Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
> Subject: iio: adc: ltc2378: Enable high-speed data capture
>
> This commit enables high-speed data capture for the ltc2378 ADC driver by
> utilizing SPI transfer offloading. It allows data acquisition at faster sample
> rates up to 2 MSPS.
>
> > diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> > index 5600c820c7f45..7d29f021cb7b4 100644
> > --- a/drivers/iio/adc/ltc2378.c
> > +++ b/drivers/iio/adc/ltc2378.c
>
> [ ... ]
>
> > + /*
> > + * Configure SPI offload PWM trigger.
> > + * The trigger should fire after tBUSYLH + tCONV + tDSDOBUSYL.
> > + * Minimum time needed: TBUSYLH (13ns) + TCONV (part-specific) + TDSDOBUSYL (5ns)
> > + *
> > + * Use the same period as CNV PWM to avoid timing issues.
> > + * Convert back from period to frequency for the SPI offload API.
> > + */
> > + offload_period_ns = cnv_wf.period_length_ns;
> > + config.periodic.frequency_hz = div_u64(HZ_PER_GHZ, offload_period_ns);
> > + min_read_offset = LTC2378_TBUSYLH_NS + st->info->tconv_ns + LTC2378_TDSDOBUSYL_NS;
> > + offload_offset_ns = min_read_offset;
> > + count = 100;
> > + do {
> > + config.periodic.offset_ns = offload_offset_ns;
> > + ret = spi_offload_trigger_validate(st->offload_trigger, &config);
> > + if (ret)
> > + return ret;
> > + offload_offset_ns += 10;
> > + } while (count-- && config.periodic.offset_ns < min_read_offset);
> > +
> > + /* Check the minimum CNV to SCLK delay is met */
> > + if (config.periodic.offset_ns < min_read_offset)
> > + return -EDOM;
> > +
> > + /* Check the PWM periods remain the same */
> > + offload_period_ns = div64_u64(HZ_PER_GHZ, config.periodic.frequency_hz);
>
> [Severity: High]
> Could this result in a kernel panic due to an integer division by zero?
>
> If a user writes 1 to the IIO in_voltage_sampling_frequency sysfs attribute,
> ltc2378_update_conversion_rate() is called with freq_Hz = 1, and the initial
> requested PWM period is calculated as 1,000,000,000 ns.
>
> If the underlying hardware PWM driver rounds the period slightly upwards
> (e.g., to 1,000,000,001 ns) during pwm_round_waveform_might_sleep() due to
> clock granularity, offload_period_ns will exceed HZ_PER_GHZ.
>
> This causes config.periodic.frequency_hz = div_u64(HZ_PER_GHZ, offload_period_ns)
> to evaluate to 0.
>
> When div64_u64(HZ_PER_GHZ, config.periodic.frequency_hz) is subsequently called,
> it divides by zero.
If we have cnv_wf.period_length_ns == NSEC_PER_SEC + 1 after the offset_ns loop
and config.periodic.frequency_hz == 0 as suggested, then spi_offload_trigger_validate()
will fail the frequency_hz check and ltc2378_update_conversion_rate() errors
out with -EINVAL before reaching div64_u64(HZ_PER_GHZ, config.periodic.frequency_hz).
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spi-offload-trigger-pwm.c?h=v7.2-rc3#n48
>
> > + if (cnv_wf.period_length_ns != offload_period_ns)
> > + return -EDOM;
> > +
> > + st->offload_trigger_config = config;
> > + st->cnv_wf = cnv_wf;
> > + st->cnv_Hz = DIV_ROUND_CLOSEST_ULL(HZ_PER_GHZ, cnv_wf.period_length_ns);
> > +
> > + return 0;
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784235595.git.marcelo.schmitt@analog.com?part=3
^ permalink raw reply
* Re: [PATCH v7 2/4] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
From: Marcelo Schmitt @ 2026-07-17 16:38 UTC (permalink / raw)
To: sashiko-reviews
Cc: Marcelo Schmitt, conor+dt, devicetree, robh, linux-iio,
marcelo.schmitt1
In-Reply-To: <20260716212838.95B3F1F000E9@smtp.kernel.org>
On 07/16, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `LTC2378_DIFF_CHANNEL` macro defines an IIO channel that is intended to be differential but lacks the `.differential = 1`, `.indexed = 1`, `.channel = 0`, and `.channel2 = 1` attributes.
> --
>
> --- Patch [2]: [PATCH v7 2/4] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs ---
> commit 5710d5f25b45906977e2522567c1964b1d3219f8
> Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
>
> iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
>
> This commit adds a new driver for the Analog Devices LTC2378-20 and
> similar analog-to-digital converters.
>
> > diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> > new file mode 100644
> > index 0000000000000..5600c820c7f45
> > --- /dev/null
> > +++ b/drivers/iio/adc/ltc2378.c
> > @@ -0,0 +1,416 @@
> [ ... ]
> > +#define LTC2378_DIFF_CHANNEL(_sign, _real_bits, _storage_bits) \
> > +{ \
> > + .type = IIO_VOLTAGE, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> > + BIT(IIO_CHAN_INFO_SCALE), \
> > + .scan_index = 0, \
>
> [Severity: Medium]
> Does this macro need to set .differential = 1, .indexed = 1, .channel = 0,
> and .channel2 = 1? The name suggests these are differential channels, but
> without these attributes, the IIO ABI will expose them as single-ended
> (e.g., in_voltage_raw instead of in_voltage0-voltage1_raw).
For parts with multiple inputs and configurable differential channels (e.g.
AD4130-8, MCP3561/2/4R, NAFE13388 [1, 2, 3]), the voltageY-voltageZ ABI helps
clarifying what physical inputs make up a channel. For differential only parts,
the ABI seems to be flexible by only requiring a 'consistent labeling' (either
voltageY or voltageY-voltageZ IIUC). Will set the channel with the differential
ABI if maintainers believe that's the best way to have it. Might slightly tweak
the channel macros to keep .differential = 0 for pseudo-differential chips
(e.g.LTC2369-18).
[1]: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4130-8.pdf
[2]: https://ww1.microchip.com/downloads/aemDocuments/documents/APID/ProductDocuments/DataSheets/MCP3561_2_4R-Data-Sheet-DS200006391C.pdf
[3]: https://www.nxp.com/docs/en/data-sheet/NAFE13388.pdf
>
> > + .scan_type = { \
> > + .format = _sign ? IIO_SCAN_FORMAT_SIGNED_INT : \
> > + IIO_SCAN_FORMAT_UNSIGNED_INT, \
> > + .realbits = _real_bits, \
> > + .storagebits = _storage_bits, \
> > + .shift = _storage_bits - _real_bits, \
> > + .endianness = IIO_BE, \
> > + }, \
> > +}
> > +
> > +#define LTC2378_BIPOLAR_DIFF_CHANNEL(_real_bits) \
> > + LTC2378_DIFF_CHANNEL(1, _real_bits, (((_real_bits) > 16) ? 32 : 16))
> > +
> > +#define LTC2378_UNIPOLAR_DIFF_CHANNEL(_real_bits) \
> > + LTC2378_DIFF_CHANNEL(0, _real_bits, (((_real_bits) > 16) ? 32 : 16))
> > +
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784235595.git.marcelo.schmitt@analog.com?part=2
^ permalink raw reply
* [PATCH v2 0/7] Properly describe mt8167 watchdog and mmsys resets
From: Luca Leonardo Scorcia @ 2026-07-17 16:39 UTC (permalink / raw)
To: linux-mediatek
Cc: Luca Leonardo Scorcia, Wim Van Sebroeck, Guenter Roeck,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-watchdog,
devicetree, linux-kernel, linux-arm-kernel
Currently mt8167 inherits its watchdog from the included mt8516 dtsi,
which in turn falls back on mt6589-wdt. However according to the data
sheet mt8167 has 15 sw resets instead of 12, and the reset bits are
different from mt6589.
In the first two patches we improve the description of the resets for
mt6589 with values obtained from Android sources.
Then we introduce a dedicated mediatek,mt8167-wdt compatible for the
watchdog driver that describes correctly this SoC's resets. Also while
we're touching the bindings reset header, we add constants for the MMSYS
resets.
In the 6th patch we add a node for the mt8167 watchdog referring to the
new compatible in the SoC dtsi.
In the last patch, we define the mmsys reset table for the SoC. According
to the datasheet, there are 28 mmsys reset bits divided across two
adjacent registers.
Changes in v2:
- Sashiko pointed out correctly a missing entry in the mmsys reset table
in the last patch.
Initial version: [1]
[1] https://lore.kernel.org/linux-mediatek/20260717151134.678839-1-l.scorcia@gmail.com/
Luca Leonardo Scorcia (7):
dt-bindings: reset: Add MT6589 toprgu reset IDs
watchdog: mediatek: Add wdt/toprgu resets for MT6589
dt-bindings: watchdog: Add compatible for MediaTek mt8167
dt-bindings: reset: Add reset controller constants for mt8167
watchdog: mediatek: Add support for mt8167 TOPRGU/WDT
arm64: dts: mt8167: Properly describe the SoC watchdog
soc: mediatek: mtk-mmsys: Add resets for mt8167
.../bindings/watchdog/mediatek,mtk-wdt.yaml | 1 +
arch/arm64/boot/dts/mediatek/mt8167.dtsi | 7 +++
drivers/soc/mediatek/mt8167-mmsys.h | 41 ++++++++++++++
drivers/soc/mediatek/mtk-mmsys.c | 3 ++
drivers/watchdog/mtk_wdt.c | 13 ++++-
.../reset/mediatek,mt6589-resets.h | 24 +++++++++
.../reset/mediatek,mt8167-resets.h | 53 +++++++++++++++++++
7 files changed, 141 insertions(+), 1 deletion(-)
create mode 100644 include/dt-bindings/reset/mediatek,mt6589-resets.h
create mode 100644 include/dt-bindings/reset/mediatek,mt8167-resets.h
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/7] dt-bindings: reset: Add MT6589 toprgu reset IDs
From: Luca Leonardo Scorcia @ 2026-07-17 16:39 UTC (permalink / raw)
To: linux-mediatek
Cc: Luca Leonardo Scorcia, Wim Van Sebroeck, Guenter Roeck,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-watchdog,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260717163959.714561-1-l.scorcia@gmail.com>
Add reset constants for the 12 MT6589 toprgu resets.
Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
---
.../reset/mediatek,mt6589-resets.h | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 include/dt-bindings/reset/mediatek,mt6589-resets.h
diff --git a/include/dt-bindings/reset/mediatek,mt6589-resets.h b/include/dt-bindings/reset/mediatek,mt6589-resets.h
new file mode 100644
index 000000000000..ee08c39df513
--- /dev/null
+++ b/include/dt-bindings/reset/mediatek,mt6589-resets.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Author: Luca Leonardo Scorcia <l.scorcia@gmail.com>
+ */
+
+#ifndef _DT_BINDINGS_RESET_CONTROLLER_MT6589
+#define _DT_BINDINGS_RESET_CONTROLLER_MT6589
+
+/* TOPRGU resets */
+#define MT6589_TOPRGU_INFRA_SW_RST 0
+#define MT6589_TOPRGU_MM_SW_RST 1
+#define MT6589_TOPRGU_MFG_SW_RST 2
+#define MT6589_TOPRGU_VENC_SW_RST 3
+#define MT6589_TOPRGU_VDEC_SW_RST 4
+#define MT6589_TOPRGU_IMG_SW_RST 5
+#define MT6589_TOPRGU_DDRPHY_SW_RST 6
+#define MT6589_TOPRGU_MD_SW_RST 7
+#define MT6589_TOPRGU_INFRA_AO_SW_RST 8
+#define MT6589_TOPRGU_MD_LITE_SW_RST 9
+#define MT6589_TOPRGU_APMIXED_SW_RST 10
+#define MT6589_TOPRGU_PWRAP_SPI_CTL_RST 11
+#define MT6589_TOPRGU_SW_RST_NUM 12
+
+#endif /* _DT_BINDINGS_RESET_CONTROLLER_MT6589 */
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/7] watchdog: mediatek: Add wdt/toprgu resets for MT6589
From: Luca Leonardo Scorcia @ 2026-07-17 16:39 UTC (permalink / raw)
To: linux-mediatek
Cc: Luca Leonardo Scorcia, Wim Van Sebroeck, Guenter Roeck,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-watchdog,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260717163959.714561-1-l.scorcia@gmail.com>
According to Android sources, MT6589 has 12 reset bits in the
WDT_SWSYSRST register. Populate toprgu_sw_rst_num to allow toprgu resets
in device trees of the many compatible devices.
Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
---
drivers/watchdog/mtk_wdt.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index 91d110646e16..e61f6ae74327 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -10,6 +10,7 @@
*/
#include <dt-bindings/reset/mt2712-resets.h>
+#include <dt-bindings/reset/mediatek,mt6589-resets.h>
#include <dt-bindings/reset/mediatek,mt6735-wdt.h>
#include <dt-bindings/reset/mediatek,mt6795-resets.h>
#include <dt-bindings/reset/mt7986-resets.h>
@@ -88,6 +89,10 @@ static const struct mtk_wdt_data mt2712_data = {
.toprgu_sw_rst_num = MT2712_TOPRGU_SW_RST_NUM,
};
+static const struct mtk_wdt_data mt6589_data = {
+ .toprgu_sw_rst_num = MT6589_TOPRGU_SW_RST_NUM,
+};
+
static const struct mtk_wdt_data mt6735_data = {
.toprgu_sw_rst_num = MT6735_TOPRGU_RST_NUM,
};
@@ -493,7 +498,7 @@ static int mtk_wdt_resume(struct device *dev)
static const struct of_device_id mtk_wdt_dt_ids[] = {
{ .compatible = "mediatek,mt2712-wdt", .data = &mt2712_data },
- { .compatible = "mediatek,mt6589-wdt" },
+ { .compatible = "mediatek,mt6589-wdt", .data = &mt6589_data },
{ .compatible = "mediatek,mt6735-wdt", .data = &mt6735_data },
{ .compatible = "mediatek,mt6795-wdt", .data = &mt6795_data },
{ .compatible = "mediatek,mt7986-wdt", .data = &mt7986_data },
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/7] dt-bindings: watchdog: Add compatible for MediaTek mt8167
From: Luca Leonardo Scorcia @ 2026-07-17 16:39 UTC (permalink / raw)
To: linux-mediatek
Cc: Luca Leonardo Scorcia, Wim Van Sebroeck, Guenter Roeck,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-watchdog,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260717163959.714561-1-l.scorcia@gmail.com>
Currently mt8167 inherits its watchdog from the included mt8516 dtsi,
which in turn falls back on mt6589-wdt. However according to the data
sheet mt8167 has 15 sw resets instead of 12, and their order is different
from mt6589. Update the wdt binding to add a dedicated compatible for
mt8167.
Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
---
Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml b/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
index 953629cb9558..f514be8a5851 100644
--- a/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
@@ -26,6 +26,7 @@ properties:
- mediatek,mt6795-wdt
- mediatek,mt7986-wdt
- mediatek,mt7988-wdt
+ - mediatek,mt8167-wdt
- mediatek,mt8183-wdt
- mediatek,mt8186-wdt
- mediatek,mt8188-wdt
--
2.43.0
^ permalink raw reply related
* [PATCH v2 4/7] dt-bindings: reset: Add reset controller constants for mt8167
From: Luca Leonardo Scorcia @ 2026-07-17 16:39 UTC (permalink / raw)
To: linux-mediatek
Cc: Luca Leonardo Scorcia, Wim Van Sebroeck, Guenter Roeck,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-watchdog,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260717163959.714561-1-l.scorcia@gmail.com>
Add the various bits that identify watchdog and mmsys resets. IDs for
mmsys resets restart from zero as they are used in a different device.
Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
---
.../reset/mediatek,mt8167-resets.h | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 include/dt-bindings/reset/mediatek,mt8167-resets.h
diff --git a/include/dt-bindings/reset/mediatek,mt8167-resets.h b/include/dt-bindings/reset/mediatek,mt8167-resets.h
new file mode 100644
index 000000000000..85d2d0e99c68
--- /dev/null
+++ b/include/dt-bindings/reset/mediatek,mt8167-resets.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+#ifndef _DT_BINDINGS_RESET_CONTROLLER_MT8167
+#define _DT_BINDINGS_RESET_CONTROLLER_MT8167
+
+/* TOPRGU resets, these are actual bits in the register */
+#define MT8167_TOPRGU_DDRPHY_FLASH_RST 0
+#define MT8167_TOPRGU_AUD_PAD_RST 1
+#define MT8167_TOPRGU_MM_RST 2
+#define MT8167_TOPRGU_MFG_RST 3
+#define MT8167_TOPRGU_MDSYS_RST 4
+#define MT8167_TOPRGU_CONN_RST 5
+#define MT8167_TOPRGU_PAD2CAM_DIG_MIPI_RX_RST 6
+#define MT8167_TOPRGU_DIG_MIPI_TX_RST 7
+#define MT8167_TOPRGU_SPI_PAD_MACRO_RST 8
+/* bit 9 is reserved, unused according to data sheet */
+#define MT8167_TOPRGU_APMIXED_RST 10
+#define MT8167_TOPRGU_VDEC_RST 11
+#define MT8167_TOPRGU_CONN_MCU_RST 12
+#define MT8167_TOPRGU_EFUSE_RST 13
+#define MT8167_TOPRGU_PWRAP_SPICTL_RST 14
+#define MT8167_TOPRGU_SW_RST_NUM 15
+
+/* MMSYS resets, these are IDs */
+#define MT8167_MMSYS_SW0_RST_B_SMI_COMMON 0
+#define MT8167_MMSYS_SW0_RST_B_SMI_LARB0 1
+#define MT8167_MMSYS_SW0_RST_B_CAM_MDP 2
+#define MT8167_MMSYS_SW0_RST_B_MDP_RDMA0 3
+#define MT8167_MMSYS_SW0_RST_B_MDP_RSZ0 4
+#define MT8167_MMSYS_SW0_RST_B_MDP_RSZ1 5
+#define MT8167_MMSYS_SW0_RST_B_MDP_TDSHP0 6
+#define MT8167_MMSYS_SW0_RST_B_MDP_WDMA 7
+#define MT8167_MMSYS_SW0_RST_B_MDP_WROT0 8
+#define MT8167_MMSYS_SW0_RST_B_FAKE_ENG 9
+#define MT8167_MMSYS_SW0_RST_B_MUTEX 10
+#define MT8167_MMSYS_SW0_RST_B_DISP_OVL0 11
+#define MT8167_MMSYS_SW0_RST_B_DISP_RDMA0 12
+#define MT8167_MMSYS_SW0_RST_B_DISP_RDMA1 13
+#define MT8167_MMSYS_SW0_RST_B_DISP_WDMA0 14
+#define MT8167_MMSYS_SW0_RST_B_DISP_COLOR 15
+#define MT8167_MMSYS_SW0_RST_B_DISP_CCORR 16
+#define MT8167_MMSYS_SW0_RST_B_DISP_AAL 17
+#define MT8167_MMSYS_SW0_RST_B_DISP_GAMMA 18
+#define MT8167_MMSYS_SW0_RST_B_DISP_DITHER 19
+#define MT8167_MMSYS_SW0_RST_B_DISP_UFOE 20
+#define MT8167_MMSYS_SW0_RST_B_DISP_PWM 21
+#define MT8167_MMSYS_SW0_RST_B_DSI0 22
+#define MT8167_MMSYS_SW0_RST_B_DPI0 23
+#define MT8167_MMSYS_SW0_RST_B_MIPI_TX_CONFIG 24
+#define MT8167_MMSYS_SW1_RST_B_LVDS_ENCODER 25
+#define MT8167_MMSYS_SW1_RST_B_DPI1 26
+#define MT8167_MMSYS_SW1_RST_B_HDMI 27
+
+#endif /* _DT_BINDINGS_RESET_CONTROLLER_MT8167 */
--
2.43.0
^ permalink raw reply related
* [PATCH v2 5/7] watchdog: mediatek: Add support for mt8167 TOPRGU/WDT
From: Luca Leonardo Scorcia @ 2026-07-17 16:39 UTC (permalink / raw)
To: linux-mediatek
Cc: Luca Leonardo Scorcia, Wim Van Sebroeck, Guenter Roeck,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-watchdog,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260717163959.714561-1-l.scorcia@gmail.com>
Add support for the Top Reset Generation Unit/Watchdog Timer found on
mt8167.
Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
---
drivers/watchdog/mtk_wdt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index e61f6ae74327..10a3d4b5ee30 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -13,6 +13,7 @@
#include <dt-bindings/reset/mediatek,mt6589-resets.h>
#include <dt-bindings/reset/mediatek,mt6735-wdt.h>
#include <dt-bindings/reset/mediatek,mt6795-resets.h>
+#include <dt-bindings/reset/mediatek,mt8167-resets.h>
#include <dt-bindings/reset/mt7986-resets.h>
#include <dt-bindings/reset/mt8183-resets.h>
#include <dt-bindings/reset/mt8186-resets.h>
@@ -110,6 +111,10 @@ static const struct mtk_wdt_data mt7988_data = {
.has_swsysrst_en = true,
};
+static const struct mtk_wdt_data mt8167_data = {
+ .toprgu_sw_rst_num = MT8167_TOPRGU_SW_RST_NUM,
+};
+
static const struct mtk_wdt_data mt8183_data = {
.toprgu_sw_rst_num = MT8183_TOPRGU_SW_RST_NUM,
};
@@ -503,6 +508,7 @@ static const struct of_device_id mtk_wdt_dt_ids[] = {
{ .compatible = "mediatek,mt6795-wdt", .data = &mt6795_data },
{ .compatible = "mediatek,mt7986-wdt", .data = &mt7986_data },
{ .compatible = "mediatek,mt7988-wdt", .data = &mt7988_data },
+ { .compatible = "mediatek,mt8167-wdt", .data = &mt8167_data },
{ .compatible = "mediatek,mt8183-wdt", .data = &mt8183_data },
{ .compatible = "mediatek,mt8186-wdt", .data = &mt8186_data },
{ .compatible = "mediatek,mt8188-wdt", .data = &mt8188_data },
--
2.43.0
^ permalink raw reply related
* [PATCH v2 6/7] arm64: dts: mt8167: Properly describe the SoC watchdog
From: Luca Leonardo Scorcia @ 2026-07-17 16:39 UTC (permalink / raw)
To: linux-mediatek
Cc: Luca Leonardo Scorcia, Wim Van Sebroeck, Guenter Roeck,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-watchdog,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260717163959.714561-1-l.scorcia@gmail.com>
Currently mt8167 inherits its watchdog from the included mt8516 dtsi,
which in turn falls back on mt6589-wdt. However according to the data
sheet mt8167 has 15 sw resets instead of 12, and the reset bits are
different from mt6589.
Use the dedicated mediatek,mt8167-wdt compatible for the watchdog driver,
as it correctly describes the SoC resets.
Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
---
arch/arm64/boot/dts/mediatek/mt8167.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8167.dtsi b/arch/arm64/boot/dts/mediatek/mt8167.dtsi
index 27cf32d7ae35..4ebf305c10fe 100644
--- a/arch/arm64/boot/dts/mediatek/mt8167.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8167.dtsi
@@ -95,6 +95,13 @@ power-domain@MT8167_POWER_DOMAIN_CONN {
};
};
+ watchdog: watchdog@10007000 {
+ compatible = "mediatek,mt8167-wdt";
+ reg = <0 0x10007000 0 0x1000>;
+ interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
+ #reset-cells = <1>;
+ };
+
pio: pinctrl@1000b000 {
compatible = "mediatek,mt8167-pinctrl";
reg = <0 0x1000b000 0 0x1000>;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 7/7] soc: mediatek: mtk-mmsys: Add resets for mt8167
From: Luca Leonardo Scorcia @ 2026-07-17 16:39 UTC (permalink / raw)
To: linux-mediatek
Cc: Luca Leonardo Scorcia, Wim Van Sebroeck, Guenter Roeck,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-watchdog,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260717163959.714561-1-l.scorcia@gmail.com>
The mt8167 SoC has 64 MMSYS resets, split in two contiguous 32-bits
registers, MMSYS_SW0_RST_B (0x140) and MMSYS_SW1_RST_B (0x144), as
also stated in the downstream kernel for the Lenovo Smart Clock
in the ddp_reg.h header.
Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
---
drivers/soc/mediatek/mt8167-mmsys.h | 41 +++++++++++++++++++++++++++++
drivers/soc/mediatek/mtk-mmsys.c | 3 +++
2 files changed, 44 insertions(+)
diff --git a/drivers/soc/mediatek/mt8167-mmsys.h b/drivers/soc/mediatek/mt8167-mmsys.h
index eef14083c47b..dc3e882a9893 100644
--- a/drivers/soc/mediatek/mt8167-mmsys.h
+++ b/drivers/soc/mediatek/mt8167-mmsys.h
@@ -3,6 +3,47 @@
#ifndef __SOC_MEDIATEK_MT8167_MMSYS_H
#define __SOC_MEDIATEK_MT8167_MMSYS_H
+#include <linux/soc/mediatek/mtk-mmsys.h>
+#include <dt-bindings/reset/mediatek,mt8167-resets.h>
+
+#define MT8167_MMSYS_SW0_RST_B 0x140
+#define MT8167_MMSYS_SW1_RST_B 0x144
+
+/* MMSYS resets */
+static const u8 mmsys_mt8167_rst_tb[] = {
+ [MT8167_MMSYS_SW0_RST_B_SMI_COMMON] = MMSYS_RST_NR(0, 0),
+ [MT8167_MMSYS_SW0_RST_B_SMI_LARB0] = MMSYS_RST_NR(0, 1),
+ [MT8167_MMSYS_SW0_RST_B_CAM_MDP] = MMSYS_RST_NR(0, 2),
+ [MT8167_MMSYS_SW0_RST_B_MDP_RDMA0] = MMSYS_RST_NR(0, 3),
+ [MT8167_MMSYS_SW0_RST_B_MDP_RSZ0] = MMSYS_RST_NR(0, 4),
+ [MT8167_MMSYS_SW0_RST_B_MDP_RSZ1] = MMSYS_RST_NR(0, 5),
+ [MT8167_MMSYS_SW0_RST_B_MDP_TDSHP0] = MMSYS_RST_NR(0, 6),
+ [MT8167_MMSYS_SW0_RST_B_MDP_WDMA] = MMSYS_RST_NR(0, 7),
+ [MT8167_MMSYS_SW0_RST_B_MDP_WROT0] = MMSYS_RST_NR(0, 8),
+ [MT8167_MMSYS_SW0_RST_B_FAKE_ENG] = MMSYS_RST_NR(0, 9),
+ [MT8167_MMSYS_SW0_RST_B_MUTEX] = MMSYS_RST_NR(0, 10),
+ [MT8167_MMSYS_SW0_RST_B_DISP_OVL0] = MMSYS_RST_NR(0, 11),
+ [MT8167_MMSYS_SW0_RST_B_DISP_RDMA0] = MMSYS_RST_NR(0, 12),
+ [MT8167_MMSYS_SW0_RST_B_DISP_RDMA1] = MMSYS_RST_NR(0, 13),
+ [MT8167_MMSYS_SW0_RST_B_DISP_WDMA0] = MMSYS_RST_NR(0, 14),
+ [MT8167_MMSYS_SW0_RST_B_DISP_COLOR] = MMSYS_RST_NR(0, 15),
+ [MT8167_MMSYS_SW0_RST_B_DISP_CCORR] = MMSYS_RST_NR(0, 16),
+ [MT8167_MMSYS_SW0_RST_B_DISP_AAL] = MMSYS_RST_NR(0, 17),
+ [MT8167_MMSYS_SW0_RST_B_DISP_GAMMA] = MMSYS_RST_NR(0, 18),
+ [MT8167_MMSYS_SW0_RST_B_DISP_DITHER] = MMSYS_RST_NR(0, 19),
+ [MT8167_MMSYS_SW0_RST_B_DISP_UFOE] = MMSYS_RST_NR(0, 20),
+ [MT8167_MMSYS_SW0_RST_B_DISP_PWM] = MMSYS_RST_NR(0, 21),
+ [MT8167_MMSYS_SW0_RST_B_DSI0] = MMSYS_RST_NR(0, 22),
+ [MT8167_MMSYS_SW0_RST_B_DPI0] = MMSYS_RST_NR(0, 23),
+ /* bit 24 is SMI_COMMON again according to data sheet */
+ /* bit 25 is SMI_LARB0 again according to data sheet */
+ [MT8167_MMSYS_SW0_RST_B_MIPI_TX_CONFIG] = MMSYS_RST_NR(0, 26),
+ /* all other bits are not described in data sheet */
+ [MT8167_MMSYS_SW1_RST_B_LVDS_ENCODER] = MMSYS_RST_NR(1, 2),
+ [MT8167_MMSYS_SW1_RST_B_DPI1] = MMSYS_RST_NR(1, 3),
+ [MT8167_MMSYS_SW1_RST_B_HDMI] = MMSYS_RST_NR(1, 4),
+};
+
#define MT8167_DISP_REG_CONFIG_DISP_OVL0_MOUT_EN 0x030
#define MT8167_DISP_REG_CONFIG_DISP_DITHER_MOUT_EN 0x038
#define MT8167_DISP_REG_CONFIG_DISP_COLOR0_SEL_IN 0x058
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 2f3e0778bb17..abd96634b63c 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -57,6 +57,9 @@ static const struct mtk_mmsys_driver_data mt8167_mmsys_driver_data = {
.clk_driver = "clk-mt8167-mm",
.routes = mt8167_mmsys_routing_table,
.num_routes = ARRAY_SIZE(mt8167_mmsys_routing_table),
+ .sw0_rst_offset = MT8167_MMSYS_SW0_RST_B,
+ .rst_tb = mmsys_mt8167_rst_tb,
+ .num_resets = ARRAY_SIZE(mmsys_mt8167_rst_tb),
};
static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v5 3/3] arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY
From: Konrad Dybcio @ 2026-07-17 16:50 UTC (permalink / raw)
To: Qiang Yu, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam,
Philipp Zabel, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260717-glymur_linkmode_0717-v5-3-4f9e87a61463@oss.qualcomm.com>
On 7/17/26 11:58 AM, Qiang Yu wrote:
> Glymur's PCIe3a and PCIe3b controllers share a single Gen5x8 QMP PHY block
> that can be bifurcated into two independent x4 links, rather than each
> controller owning its own dedicated PHY.
>
> Add a pcie3_phy node describing the shared PHY block, add the missing
> PCIe3a controller node, and point both PCIe3a's and PCIe3b's port phys
> at &pcie3_phy (index 0 and 1 respectively) so each controller picks up
> its half of the bifurcated PHY. Update the GCC pipe clock parent array
> to reference the new PHY's clock outputs instead of the placeholders.
>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/glymur-crd.dtsi | 8 +
> arch/arm64/boot/dts/qcom/glymur.dtsi | 336 ++++++++++++++++++++++++++++++-
These changes really shouldn't be in the same commit
> 2 files changed, 342 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> index c12808abbfe1..e8e81bf9a6a1 100644
> --- a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> @@ -750,6 +750,14 @@ &pcie3b {
> pinctrl-names = "default";
> };
>
> +&pcie3_phy {
> + vdda-phy-supply = <&vreg_l3c_e1_0p89>;
> + vdda-pll-supply = <&vreg_l2c_e1_1p14>;
> +
> + vdda-refgen0p9-supply = <&vreg_l1c_e1_0p82>;
> + vdda-refgen1p2-supply = <&vreg_l4f_e1_1p08>;
> +};
This node is disabled
Konrad
^ permalink raw reply
* Re: [PATCH v4 1/7] dt-bindings: clock: Introduce nexus nodes
From: sashiko-bot @ 2026-07-17 16:51 UTC (permalink / raw)
To: Miquel Raynal (Schneider Electric); +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260717-schneider-v7-2-rc1-eip201-upstream-v4-1-751547e160e5@bootlin.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The clock nexus node schema omits standard nexus properties (`clock-map-mask`, `clock-map-pass-thru`) and `#clock-cells`, breaking schema validation.
- [Medium] The binding schema entirely omits the mandatory `examples` section.
--
--- Patch [1]: [PATCH v4 1/7] dt-bindings: clock: Introduce nexus nodes ---
commit f0afb8c47352e777825bd7597a7b17705e20153d
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
dt-bindings: clock: Introduce nexus nodes
This commit introduces a new device tree binding for clock nexus nodes
to allow remapping of clock specifiers. It defines the clock-map property
but explicitly defers examples to the existing PWM nexus node binding.
> diff --git a/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml b/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml
> new file mode 100644
> index 0000000000000..ff3283bd5fb11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml
[ ... ]
> +properties:
> + clock-map:
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +
> +dependentRequired:
> + clock-map: ['#clock-cells']
> +
> +additionalProperties: true
[Severity: Medium]
The commit message states this is a literal copy-paste from the PWM nexus
node binding, but should this properties block also include the standard
nexus properties like clock-map-mask, clock-map-pass-thru, and #clock-cells?
Without these definitions and their corresponding dependentRequired constraints,
schema validation won't enforce type checks on the mask and pass-thru
properties, and won't ensure that clock-map is present when a mask is used.
> +
> +# See the original pwm-nexus-node.yaml description for examples
[Severity: Medium]
Does omitting the examples section bypass the automated dt_binding_check
tests for this specific binding?
The validation tooling uses the examples section to generate test device
trees to verify logical and syntactical constraints. Providing an example
here would ensure the schema is properly tested by the build tools.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com?part=1
^ permalink raw reply
* Re: [PATCH v4 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Dmitry Torokhov @ 2026-07-17 16:51 UTC (permalink / raw)
To: Fenglin Wu
Cc: linux-arm-msm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Stephen Boyd, Bjorn Andersson, Konrad Dybcio,
David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260717-qcom-spmi-haptics-v4-3-b0fe0ed30849@oss.qualcomm.com>
Hi Fenglin,
On Fri, Jul 17, 2026 at 12:29:01AM -0700, Fenglin Wu wrote:
> Add an initial driver for the Qualcomm PMIH0108 PMIC haptics module,
> named as HAP530_HV. This module supports several play modes, including
> DIRECT_PLAY, FIFO, PAT_MEM, and SWR, each with distinct data sourcing
> and hardware data handling logic. Currently, the driver provides support
> for two play modes using the input force-feedback framework: FF_CONSTANT
> effect for DIRECT_PLAY mode and FF_PERIODIC effect with FF_CUSTOM
> waveform for FIFO mode.
>
> Assisted-by: Claude:claude-4-8-opus
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
> drivers/input/misc/Kconfig | 11 +
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/qcom-spmi-haptics.c | 1178 ++++++++++++++++++++++++++++++++
> 3 files changed, 1190 insertions(+)
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 1f6c57dba030..4f40940973e4 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -236,6 +236,17 @@ config INPUT_PMIC8XXX_PWRKEY
> To compile this driver as a module, choose M here: the
> module will be called pmic8xxx-pwrkey.
>
> +config INPUT_QCOM_SPMI_HAPTICS
> + tristate "Qualcomm SPMI PMIC haptics support"
> + depends on MFD_SPMI_PMIC
> + help
> + Say Y to enable support for the Qualcomm PMIH0108 SPMI PMIC haptics
> + module. Supports DIRECT_PLAY, FIFO streaming play modes via the
> + Linux input force-feedback framework.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called qcom-spmi-haptics.
> +
> config INPUT_SPARCSPKR
> tristate "SPARC Speaker support"
> depends on PCI && SPARC64
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 2281d6803fce..c5c9aa139a11 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
> obj-$(CONFIG_INPUT_PWM_VIBRA) += pwm-vibra.o
> +obj-$(CONFIG_INPUT_QCOM_SPMI_HAPTICS) += qcom-spmi-haptics.o
> obj-$(CONFIG_INPUT_QNAP_MCU) += qnap-mcu-input.o
> obj-$(CONFIG_INPUT_RAVE_SP_PWRBUTTON) += rave-sp-pwrbutton.o
> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
> diff --git a/drivers/input/misc/qcom-spmi-haptics.c b/drivers/input/misc/qcom-spmi-haptics.c
> new file mode 100644
> index 000000000000..235f77e0229c
> --- /dev/null
> +++ b/drivers/input/misc/qcom-spmi-haptics.c
> @@ -0,0 +1,1178 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/list.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/uaccess.h>
> +#include <linux/workqueue.h>
> +
> +/* HAP_CFG register offsets, bit fields, value constants */
> +#define HAP_CFG_INT_RT_STS_REG 0x10
> +#define FIFO_EMPTY_BIT BIT(1)
> +#define HAP_CFG_EN_CTL_REG 0x46
> +#define HAPTICS_EN_BIT BIT(7)
> +#define HAP_CFG_VMAX_REG 0x48
> +#define VMAX_STEP_MV 50
> +#define VMAX_MV_MAX 10000
> +#define HAP_CFG_SPMI_PLAY_REG 0x4C
> +#define PLAY_EN_BIT BIT(7)
> +#define PAT_SRC_MASK GENMASK(2, 0)
> +#define PAT_SRC_FIFO 0
> +#define PAT_SRC_DIRECT_PLAY 1
> +#define HAP_CFG_TLRA_OL_HIGH_REG 0x5C
> +#define TLRA_OL_MSB_MASK GENMASK(3, 0)
> +#define TLRA_STEP_US 5
> +#define TLRA_US_MAX 20475
> +#define HAP_CFG_TLRA_OL_LOW_REG 0x5D
> +#define HAP_CFG_DRV_DUTY_CFG_REG 0x60
> +#define ADT_DRV_DUTY_EN_BIT BIT(7)
> +#define ADT_BRK_DUTY_EN_BIT BIT(6)
> +#define DRV_DUTY_MASK GENMASK(5, 3)
> +#define AUTORES_DRV_DUTY_62P5 2
> +#define BRK_DUTY_MASK GENMASK(2, 0)
> +#define AUTORES_BRK_DUTY_62P5 5
> +#define HAP_CFG_ZX_WIND_CFG_REG 0x62
> +#define ZX_DEBOUNCE_MASK GENMASK(6, 4)
> +#define AUTORES_ZX_DEBOUNCE 3
> +#define ZX_WIN_HEIGHT_MASK GENMASK(2, 0)
> +#define AUTORES_ZX_WIN_HEIGHT 2
> +#define HAP_CFG_AUTORES_CFG_REG 0x63
> +#define AUTORES_EN_BIT BIT(7)
> +#define AUTORES_EN_DLY_MASK GENMASK(6, 2)
> +#define AUTORES_EN_DLY_CYCLES 10
> +#define AUTORES_ERR_WIN_MASK GENMASK(1, 0)
> +#define AUTORES_ERR_WIN_25PCT 1
> +#define HAP_CFG_FAULT_CLR_REG 0x66
> +#define ZX_TO_FAULT_CLR_BIT BIT(4)
> +#define SC_CLR_BIT BIT(2)
> +#define AUTO_RES_ERR_CLR_BIT BIT(1)
> +#define HPWR_RDY_FAULT_CLR_BIT BIT(0)
> +#define FAULT_CLR_ALL (ZX_TO_FAULT_CLR_BIT | SC_CLR_BIT | \
> + AUTO_RES_ERR_CLR_BIT | HPWR_RDY_FAULT_CLR_BIT)
> +#define HAP_CFG_RAMP_DN_CFG2_REG 0x86
> +#define AUTORES_PRE_HIZ_DLY_10US 1
> +
> +/* HAP_PTN register offsets, bit fields, value constants */
> +#define HAP_PTN_REVISION2_REG 0x01
> +#define HAP_PTN_FIFO_DIN_0_REG 0x20
> +#define HAP_PTN_FIFO_PLAY_RATE_REG 0x24
> +#define FIFO_PLAY_RATE_MASK GENMASK(3, 0)
> +#define HAP_PTN_DIRECT_PLAY_REG 0x26
> +#define HAP_PTN_FIFO_EMPTY_CFG_REG 0x2A
> +#define FIFO_THRESH_LSB 64
> +#define HAP_PTN_FIFO_DIN_1B_REG 0x2C
> +#define HAP_PTN_MEM_OP_ACCESS_REG 0x2D
> +#define MEM_FLUSH_RELOAD_BIT BIT(0)
> +#define HAP_PTN_MMAP_FIFO_REG 0xA0
> +#define MMAP_FIFO_EXIST_BIT BIT(7)
> +#define MMAP_FIFO_LEN_MASK GENMASK(6, 0)
> +#define HAP_PTN_PATX_PLAY_CFG_REG 0xA2
> +
> +#define HAP530_MEM_TOTAL_BYTES 8192
> +#define FIFO_EMPTY_THRESH 280
> +#define FIFO_INIT_FILL 320
> +
> +#define HAPTICS_AUTOSUSPEND_MS 1000
> +
> +/*
> + * FF_CUSTOM data layout (custom_data[] of type s16):
> + * [0] = play rate (PLAY_RATE_*)
> + * [1] = vmax in mV (0 = use device default from qcom,vmax-microvolt)
> + * [2..N-1] = signed 8-bit PCM samples packed one per s16 (lower byte used)
> + */
> +#define CUSTOM_DATA_RATE_IDX 0
> +#define CUSTOM_DATA_VMAX_IDX 1
> +#define CUSTOM_DATA_SAMPLE_START 2
> +#define CUSTOM_DATA_MAX_LEN (48 * 1024)
> +
> +#define HAPTICS_MAX_EFFECTS 8
> +
> +enum qcom_haptics_mode {
> + HAPTICS_MODE_NONE,
> + HAPTICS_DIRECT_PLAY,
> + HAPTICS_FIFO,
> +};
> +
> +enum qcom_haptics_play_rate {
> + PLAY_RATE_T_LRA = 0,
> + PLAY_RATE_T_LRA_DIV_2 = 1,
> + PLAY_RATE_T_LRA_DIV_4 = 2,
> + PLAY_RATE_T_LRA_DIV_8 = 3,
> + PLAY_RATE_T_LRA_X_2 = 4,
> + PLAY_RATE_T_LRA_X_4 = 5,
> + PLAY_RATE_T_LRA_X_8 = 6,
> + PLAY_RATE_RESERVED = 7,
> + PLAY_RATE_F_8KHZ = 8,
> + PLAY_RATE_F_16KHZ = 9,
> + PLAY_RATE_F_24KHZ = 10,
> + PLAY_RATE_F_32KHZ = 11,
> + PLAY_RATE_F_44P1KHZ = 12,
> + PLAY_RATE_F_48KHZ = 13,
> + PLAY_RATE_MAX = PLAY_RATE_F_48KHZ,
> +};
> +
> +/**
> + * struct qcom_haptics_effect: A haptics effect
> + * @mode: haptics HW play mode
> + * @vmax_mv: peak voltage of the haptics output waveform
> + * @length_us: vibration play duration
> + * @amplitude: DIRECT_PLAY mode output waveform amplitude
> + * @play_rate: FIFO mode play rate
> + * @fifo_data: 8-bit data samples consumed in FIFO mode
> + * @data_len: length of the FIFO data samples
> + */
> +struct qcom_haptics_effect {
> + enum qcom_haptics_mode mode;
> + u32 vmax_mv;
> + u32 length_us;
> +
> + u8 amplitude;
> +
> + enum qcom_haptics_play_rate play_rate;
> + s8 *fifo_data;
> + u32 data_len;
> +};
> +
> +/**
> + * struct haptics_play_req: A haptics play request
> + * @node: list node of the request
> + * @effect_id: effect index of the request
> + * @play: flag of starting or stopping the play
> + */
> +struct haptics_play_req {
> + struct list_head node;
> + int effect_id;
> + bool play;
> +};
> +
> +/**
> + * struct qcom_haptics
> + * @dev: underlying SPMI device
> + * @regmap: regmap for SPMI register access
> + * @input: input device exposing the FF interface
> + * @cfg_base: base address of the CFG peripheral
> + * @ptn_base: base address of the PTN peripheral
> + * @t_lra_us: LRA resonance period in microseconds
> + * @vmax_mv: maximum actuator drive voltage in millivolts
> + * @fifo_len: programmed HW FIFO depth in bytes
> + * @gain: playback gain scaler
> + * @play_work: delayed work that plays the queued requests
> + * @play_lock: mutex lock to serialize playbacks
> + * @play_queue_lock: spinlock protecting @play_queue
> + * @play_queue: The list of pending start/stop requests
> + * @active_effect_id: index into @effects[] currently under play
> + * @active_mode: mode of the effect currently armed in hardware
> + * @fifo_empty_irq: IRQ number for the FIFO-empty interrupt
> + * @pm_ref_held: true while a pm_runtime_get is held
> + * @irq_enabled: true if fifo_empty_irq is enabled
> + * @fifo_lock: mutex protecting the FIFO streaming data
> + * @fifo_data: pointer of the data buffer for FIFO streaming
> + * @data_len: length of the data buffer for current effect
> + * @data_written: number of samples written to the hardware FIFO
> + * @effects: table of the effects
> + */
> +struct qcom_haptics {
> + struct device *dev;
> + struct regmap *regmap;
> + struct input_dev *input;
> +
> + u32 cfg_base;
> + u32 ptn_base;
> + u32 t_lra_us;
> + u32 vmax_mv;
> + u32 fifo_len;
> + atomic_t gain;
> +
> + struct delayed_work play_work;
> + struct mutex play_lock; /* mutex used to serialize playbacks */
> +
> + spinlock_t play_queue_lock; /* protects play_queue */
> + struct list_head play_queue;
> +
> + int active_effect_id;
> + enum qcom_haptics_mode active_mode;
> +
> + int fifo_empty_irq;
> + bool pm_ref_held;
> + bool irq_enabled;
> +
> + struct mutex fifo_lock; /* protect the FIFO data during play */
> + const s8 *fifo_data;
> + u32 data_len;
> + u32 data_written;
> +
> + struct qcom_haptics_effect effects[HAPTICS_MAX_EFFECTS];
> +};
> +
> +static int cfg_write(struct qcom_haptics *h, u32 off, u32 val)
> +{
> + return regmap_write(h->regmap, h->cfg_base + off, val);
> +}
> +
> +static int cfg_update_bits(struct qcom_haptics *h, u32 off, u32 mask, u32 val)
> +{
> + return regmap_update_bits(h->regmap, h->cfg_base + off, mask, val);
> +}
> +
> +static int ptn_write(struct qcom_haptics *h, u32 off, u32 val)
> +{
> + return regmap_write(h->regmap, h->ptn_base + off, val);
> +}
> +
> +static int ptn_update_bits(struct qcom_haptics *h, u32 off, u32 mask, u32 val)
> +{
> + return regmap_update_bits(h->regmap, h->ptn_base + off, mask, val);
> +}
> +
> +static int ptn_bulk_write(struct qcom_haptics *h, u32 off,
> + const void *buf, size_t count)
> +{
> + return regmap_bulk_write(h->regmap, h->ptn_base + off, buf, count);
> +}
> +
> +static int haptics_clear_faults(struct qcom_haptics *h)
> +{
> + return cfg_write(h, HAP_CFG_FAULT_CLR_REG, FAULT_CLR_ALL);
> +}
> +
> +static int haptics_set_vmax(struct qcom_haptics *h, u32 vmax_mv)
> +{
> + return cfg_write(h, HAP_CFG_VMAX_REG, vmax_mv / VMAX_STEP_MV);
> +}
> +
> +static int haptics_config_lra_period(struct qcom_haptics *h)
> +{
> + u32 tmp = h->t_lra_us / TLRA_STEP_US;
> + int ret;
Please use "error" or "err" for such variables that hold error codes.
> +
> + ret = cfg_write(h, HAP_CFG_TLRA_OL_HIGH_REG, (tmp >> 8) & TLRA_OL_MSB_MASK);
> + if (ret)
> + return ret;
> +
> + return cfg_write(h, HAP_CFG_TLRA_OL_LOW_REG, tmp & 0xFF);
Explicit error handling and "return 0;" in cases where there are
multiple failure points.
> +}
> +
> +static int haptics_enable_module(struct qcom_haptics *h, bool enable)
> +{
> + return cfg_update_bits(h, HAP_CFG_EN_CTL_REG, HAPTICS_EN_BIT,
> + enable ? HAPTICS_EN_BIT : 0);
> +}
> +
> +static int haptics_configure_autores(struct qcom_haptics *h)
> +{
> + int ret;
> +
> + /* AUTORES_CFG: enable, 10-cycle delay, 25% error window */
> + ret = cfg_write(h, HAP_CFG_AUTORES_CFG_REG,
> + AUTORES_EN_BIT |
> + FIELD_PREP(AUTORES_EN_DLY_MASK, AUTORES_EN_DLY_CYCLES) |
> + FIELD_PREP(AUTORES_ERR_WIN_MASK, AUTORES_ERR_WIN_25PCT));
> + if (ret)
> + return ret;
> +
> + /* DRV_DUTY: adaptive drive/brake duty cycles at 62.5% */
> + ret = cfg_write(h, HAP_CFG_DRV_DUTY_CFG_REG,
> + ADT_DRV_DUTY_EN_BIT | ADT_BRK_DUTY_EN_BIT |
> + FIELD_PREP(DRV_DUTY_MASK, AUTORES_DRV_DUTY_62P5) |
> + FIELD_PREP(BRK_DUTY_MASK, AUTORES_BRK_DUTY_62P5));
> + if (ret)
> + return ret;
> +
> + /* Pre-HIZ delay: 10 µs */
> + ret = cfg_write(h, HAP_CFG_RAMP_DN_CFG2_REG, AUTORES_PRE_HIZ_DLY_10US);
> + if (ret)
> + return ret;
> +
> + /* Zero-cross window: debounce 3, no hysteresis, height 2 */
> + return cfg_write(h, HAP_CFG_ZX_WIND_CFG_REG,
> + FIELD_PREP(ZX_DEBOUNCE_MASK, AUTORES_ZX_DEBOUNCE) |
> + FIELD_PREP(ZX_WIN_HEIGHT_MASK, AUTORES_ZX_WIN_HEIGHT));
> +}
> +
> +static int haptics_write_fifo_chunk(struct qcom_haptics *h,
> + const s8 *data, u32 len)
> +{
> + u32 bulk_len = ALIGN_DOWN(len, 4);
> + int i, ret;
> +
> + /*
> + * FIFO data writing supports both 4-byte bulk writes using registers
> + * [HAP_PTN_FIFO_DIN_0_REG ... HAP_PTN_FIFO_DIN_3_REG], and 1-byte writes
> + * using the HAP_PTN_FIFO_DIN_1B_REG register. The 4-byte bulk write is more
> + * efficient, so use 4-byte writes for the initial 4-byte aligned data,
> + * and 1-byte writes for any trailing remainder.
> + */
> + for (i = 0; i < bulk_len; i += 4) {
> + ret = ptn_bulk_write(h, HAP_PTN_FIFO_DIN_0_REG, &data[i], 4);
> + if (ret)
> + return ret;
> + }
> +
> + for (; i < len; i++) {
> + ret = ptn_write(h, HAP_PTN_FIFO_DIN_1B_REG, (u8)data[i]);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int haptics_configure_fifo_mmap(struct qcom_haptics *h)
> +{
> + u32 fifo_len, fifo_units;
> +
> + /* Config all memory space for FIFO usage for now */
> + fifo_len = HAP530_MEM_TOTAL_BYTES;
> + fifo_len = ALIGN_DOWN(fifo_len, 64);
> + fifo_units = fifo_len / 64;
> + h->fifo_len = fifo_len;
> +
> + return ptn_write(h, HAP_PTN_MMAP_FIFO_REG,
> + MMAP_FIFO_EXIST_BIT |
> + FIELD_PREP(MMAP_FIFO_LEN_MASK, fifo_units - 1));
> +}
> +
> +static u32 haptics_gain_scaled_vmax(struct qcom_haptics *h, u32 vmax_mv)
> +{
> + u16 gain = atomic_read(&h->gain);
> + u32 v = mult_frac(vmax_mv, gain, 0xFFFF);
> +
> + return max_t(u32, v, VMAX_STEP_MV);
> +}
> +
> +static void haptics_fifo_irq_enable(struct qcom_haptics *h, bool enable)
> +{
> + if (h->irq_enabled == enable)
> + return;
Should t you know if given code runs with interrupts disabled or
enabled? I believe this tracking and the wrapper should be removed.
> +
> + if (enable)
> + enable_irq(h->fifo_empty_irq);
> + else
> + disable_irq(h->fifo_empty_irq);
> +
> + h->irq_enabled = enable;
> +}
> +
> +static int haptics_enqueue_play_req(struct qcom_haptics *h, int effect_id, bool play)
> +{
> + struct haptics_play_req *req;
> +
> + guard(spinlock_irqsave)(&h->play_queue_lock);
> +
> + /*
> + * Coalesce with an already queued request for
> + * the same effect instead of appending.
> + */
> + list_for_each_entry(req, &h->play_queue, node) {
> + if (req->effect_id == effect_id) {
> + req->play = play;
> + return 0;
> + }
> + }
> +
> + req = kmalloc_obj(*req, GFP_ATOMIC);
> + if (!req)
> + return -ENOMEM;
> +
> + req->effect_id = effect_id;
> + req->play = play;
> +
> + list_add_tail(&req->node, &h->play_queue);
> +
> + return 0;
> +}
> +
> +static struct haptics_play_req *haptics_dequeue_play_req(struct qcom_haptics *h)
> +{
> + struct haptics_play_req *req = NULL;
> +
> + guard(spinlock_irqsave)(&h->play_queue_lock);
> +
> + if (!list_empty(&h->play_queue)) {
> + req = list_first_entry(&h->play_queue, struct haptics_play_req, node);
> + list_del(&req->node);
> + }
> +
> + return req;
> +}
> +
> +static bool haptics_queue_pending(struct qcom_haptics *h)
> +{
> + guard(spinlock_irqsave)(&h->play_queue_lock);
> +
> + return !list_empty(&h->play_queue);
I strongly dislike this kind of functions since their result is
obsolete the moment they return. Sometimes it is OK, but usually it
isn't. In this driver I think it is fine, but I would much rather inline
this into the caller and added comment. We can also schedule work
holding this spinlock I think...
> +}
> +
> +static void haptics_queue_remove_effect(struct qcom_haptics *h, int effect_id)
> +{
> + struct haptics_play_req *req, *tmp;
> +
> + guard(spinlock_irqsave)(&h->play_queue_lock);
> +
> + list_for_each_entry_safe(req, tmp, &h->play_queue, node) {
> + if (req->effect_id == effect_id) {
> + list_del(&req->node);
> + kfree(req);
> + }
> + }
> +}
> +
> +static void haptics_queue_flush(struct qcom_haptics *h)
> +{
> + struct haptics_play_req *req, *tmp;
> +
> + guard(spinlock_irqsave)(&h->play_queue_lock);
> +
> + list_for_each_entry_safe(req, tmp, &h->play_queue, node) {
> + list_del(&req->node);
> + kfree(req);
> + }
> +}
> +
> +static int haptics_start_direct_play(struct qcom_haptics *h, int effect_id)
> +{
> + struct qcom_haptics_effect *eff = &h->effects[effect_id];
> + int ret;
> +
> + ret = haptics_clear_faults(h);
> + if (ret)
> + return ret;
> +
> + /* Enable auto-resonance for DIRECT_PLAY mode */
> + ret = cfg_update_bits(h, HAP_CFG_AUTORES_CFG_REG,
> + AUTORES_EN_BIT, AUTORES_EN_BIT);
> + if (ret)
> + return ret;
> +
> + ret = haptics_set_vmax(h, haptics_gain_scaled_vmax(h, h->vmax_mv));
> + if (ret)
> + return ret;
> +
> + ret = ptn_write(h, HAP_PTN_DIRECT_PLAY_REG, eff->amplitude);
> + if (ret)
> + return ret;
> +
> + return cfg_write(h, HAP_CFG_SPMI_PLAY_REG,
> + PLAY_EN_BIT | FIELD_PREP(PAT_SRC_MASK, PAT_SRC_DIRECT_PLAY));
> +}
> +
> +static int haptics_start_fifo(struct qcom_haptics *h, int effect_id)
> +{
> + struct qcom_haptics_effect *eff = &h->effects[effect_id];
> + u32 vmax = eff->vmax_mv ? eff->vmax_mv : h->vmax_mv;
> + u32 init_len;
> + bool data_done;
> + int ret;
> +
> + if (!eff->fifo_data || !eff->data_len)
> + return -EINVAL;
> +
> + ret = haptics_clear_faults(h);
> + if (ret)
> + return ret;
> +
> + /* Disable auto-resonance for FIFO mode */
> + ret = cfg_update_bits(h, HAP_CFG_AUTORES_CFG_REG, AUTORES_EN_BIT, 0);
> + if (ret)
> + return ret;
> +
> + ret = haptics_set_vmax(h, haptics_gain_scaled_vmax(h, vmax));
> + if (ret)
> + return ret;
> +
> + ret = ptn_update_bits(h, HAP_PTN_FIFO_PLAY_RATE_REG,
> + FIFO_PLAY_RATE_MASK,
> + FIELD_PREP(FIFO_PLAY_RATE_MASK, eff->play_rate));
> + if (ret)
> + return ret;
> +
> + /* Flush FIFO before loading new data */
> + ret = ptn_write(h, HAP_PTN_MEM_OP_ACCESS_REG, MEM_FLUSH_RELOAD_BIT);
> + if (ret)
> + return ret;
> + ret = ptn_write(h, HAP_PTN_MEM_OP_ACCESS_REG, 0);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&h->fifo_lock);
> +
> + /* Write the initial chunk and initialise streaming state */
> + init_len = min_t(u32, eff->data_len, FIFO_INIT_FILL);
> + ret = haptics_write_fifo_chunk(h, eff->fifo_data, init_len);
> + if (ret)
> + return ret;
> +
> + h->fifo_data = eff->fifo_data;
> + h->data_len = eff->data_len;
> + h->data_written = init_len;
> +
> + /*
> + * Set empty threshold. When threshold > 0 the hardware fires the
> + * FIFO-empty interrupt when occupancy drops below the threshold,
> + * allowing the driver to refill.
> + */
> + data_done = (h->data_written >= h->data_len);
> + if (!data_done) {
> + ret = ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG,
> + FIFO_EMPTY_THRESH / FIFO_THRESH_LSB);
> + if (ret) {
> + dev_err(h->dev, "set FIFO empty threshold failed, ret=%d\n", ret);
> + h->fifo_data = NULL;
> + return ret;
> + }
> +
> + haptics_fifo_irq_enable(h, true);
> + }
> +
> + ret = cfg_write(h, HAP_CFG_SPMI_PLAY_REG,
> + PLAY_EN_BIT | FIELD_PREP(PAT_SRC_MASK, PAT_SRC_FIFO));
> + if (ret) {
> + dev_err(h->dev, "trigger FIFO play failed, ret=%d\n", ret);
> + ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
> + /*
> + * HW play never started since this SPMI write itself failed,
> + * and the FIFO empty IRQ has never fired, so disabling the
> + * IRQ synchronously here can't deadlock against 'fifo_lock'.
> + */
> + haptics_fifo_irq_enable(h, false);
> + h->fifo_data = NULL;
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * haptics_fifo_empty_irq: Threaded IRQ handler for the FIFO-empty interrupt.
> + *
> + * While a FIFO play is in progress the hardware fires this interrupt when
> + * the number of samples in the FIFO drops below the programmed threshold.
> + * The handler refills the FIFO from the effect's data buffer. When all
> + * samples have been written the threshold is set to zero. The HW would
> + * stop the play automatically after all of the samples in FIFO memory are
> + * played out.
> + */
> +static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
> +{
> + struct qcom_haptics *h = dev_id;
> + u32 sts, to_write;
> + int ret;
> +
> + ret = regmap_read(h->regmap,
> + h->cfg_base + HAP_CFG_INT_RT_STS_REG, &sts);
> + if (ret || !(sts & FIFO_EMPTY_BIT))
> + return IRQ_HANDLED;
> +
> + guard(mutex)(&h->fifo_lock);
> +
> + if (!h->fifo_data)
> + return IRQ_HANDLED;
> +
> + /* Refill: write the next chunk */
> + to_write = min_t(u32, h->data_len - h->data_written,
> + h->fifo_len - FIFO_EMPTY_THRESH);
> + ret = haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
> + if (ret) {
> + dev_err(h->dev, "refill FIFO samples failed, ret=%d\n", ret);
> + /*
> + * If data refilling is failed,stop the HW play and disable the
> + * IRQ to prevent the FIFO empty IRQ being fired continuously.
> + */
Is recovery possible after this?
> + cfg_write(h, HAP_CFG_SPMI_PLAY_REG, 0);
> + disable_irq_nosync(h->fifo_empty_irq);
> + h->irq_enabled = false;
> + return IRQ_HANDLED;
> + }
> +
> + h->data_written += to_write;
> +
> + /* Disable the interrupt after all the data is queued */
> + if (h->data_written >= h->data_len)
> + ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/*
> + * haptics_stop_locked: stop play in HW and put runtime PM
> + * @h: haptics device
> + * @put_noidle: a flag to put noidle
> + *
> + * Stop HW play, clear the FIFO data if the active effect was FIFO-mode, and
> + * put runtime PM into either noidle or autosuspend based on put_noidle flag.
> + *
> + * Must be called with play_lock held.
Maybe add lockdep_assert_held() instead of comments/naming?
> + */
> +static void haptics_stop_locked(struct qcom_haptics *h, bool put_noidle)
> +{
> + cfg_write(h, HAP_CFG_SPMI_PLAY_REG, 0);
> +
> + if (h->active_mode == HAPTICS_FIFO) {
> + ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
> + haptics_fifo_irq_enable(h, false);
> + scoped_guard(mutex, &h->fifo_lock) {
> + h->fifo_data = NULL;
> + }
> + }
> +
> + h->active_effect_id = -1;
> + h->active_mode = HAPTICS_MODE_NONE;
> +
> + if (h->pm_ref_held) {
> + if (put_noidle)
> + pm_runtime_put_noidle(h->dev);
> + else
> + pm_runtime_put_autosuspend(h->dev);
> +
> + h->pm_ref_held = false;
> + }
> +}
> +
> +/*
> + * haptics_start_locked: start to play an effect
> + * @h: haptics device
> + * @effect_id: the index of the effect
> + *
> + * If an effect is currently active, stop it 1st. Acquires a PM ref
> + * if not already held, and then trigger the play based on the
> + * play mode. Update active_effect_id/active_mode on success.
> + *
> + * Must be called with play_lock held.
> + */
> +static int haptics_start_locked(struct qcom_haptics *h, int effect_id)
> +{
> + int ret;
> +
> + if (h->active_effect_id != -1)
> + haptics_stop_locked(h, false);
> +
> + if (!h->pm_ref_held) {
> + ret = pm_runtime_resume_and_get(h->dev);
> + if (ret < 0) {
> + dev_err(h->dev, "failed to resume device: %d\n", ret);
> + return ret;
> + }
> +
> + h->pm_ref_held = true;
> + }
> +
> + switch (h->effects[effect_id].mode) {
> + case HAPTICS_DIRECT_PLAY:
> + ret = haptics_start_direct_play(h, effect_id);
> + break;
> + case HAPTICS_FIFO:
> + ret = haptics_start_fifo(h, effect_id);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + if (ret) {
> + dev_err(h->dev, "failed to start effect %d: %d\n", effect_id, ret);
> + if (h->pm_ref_held) {
> + pm_runtime_put_autosuspend(h->dev);
> + h->pm_ref_held = false;
> + }
> +
> + return ret;
> + }
> +
> + h->active_effect_id = effect_id;
> + h->active_mode = h->effects[effect_id].mode;
> +
> + return 0;
> +}
> +
> +static void haptics_play_work(struct work_struct *work)
> +{
> + struct qcom_haptics *h = container_of(to_delayed_work(work),
> + struct qcom_haptics, play_work);
> + struct haptics_play_req *req = haptics_dequeue_play_req(h);
> + bool rearmed = false;
> + u32 length_us;
> + int ret;
> +
> + guard(mutex)(&h->play_lock);
> +
> + /* No valid request available */
> + if (req == NULL)
if (!req)
> + return;
> +
> + if (req->play) {
> + ret = haptics_start_locked(h, req->effect_id);
> + if (ret) {
> + dev_err(h->dev, "play haptics failed, ret=%d\n", ret);
> + } else {
> + /*
> + * Re-arm the work to stop the play or advance
> + * to next play after the play duration
> + */
> + length_us = h->effects[req->effect_id].length_us;
> + if (length_us) {
> + schedule_delayed_work(&h->play_work,
> + usecs_to_jiffies(length_us));
> + rearmed = true;
> + }
> + }
> + } else {
> + if (req->effect_id != h->active_effect_id)
> + dev_warn(h->dev, "effect %d is not under playing\n", req->effect_id);
> + else
> + haptics_stop_locked(h, false);
> + }
> +
> + kfree(req);
> +
> + /*
> + * Handle the remaining requests if doesn't need to wait
> + */
> + if (!rearmed && haptics_queue_pending(h))
> + schedule_delayed_work(&h->play_work, 0);
> +}
> +
> +static void haptics_clear_effect(struct qcom_haptics *h,
> + struct qcom_haptics_effect *effect)
> +{
> + guard(mutex)(&h->fifo_lock);
> +
> + if (h->fifo_data == effect->fifo_data)
> + h->fifo_data = NULL;
> +
> + kvfree(effect->fifo_data);
> + effect->fifo_data = NULL;
> + effect->data_len = 0;
> +}
> +
> +/*
> + * haptics_fifo_length_us: Calculate the play duration of a FIFO effect.
> + * @h: haptics device pointer
> + * @rate: FIFO data play rate
> + * @data_len: FFO data length
> + *
> + * Each FIFO sample is played out over one play-rate period, so the total
> + * duration is the number of samples times that period:
> + *
> + * length_us = (clk_base * data_len * multiplier) / divider
> + *
> + * For T_LRA-based rates the period derives from the LRA resonance period
> + * (h->t_lra_us): DIV_2/4/8 shorten it, X_2/4/8 lengthen it. For the
> + * kHz-based rates the period is 1000 us / freq_khz.
> + */
> +static u32 haptics_fifo_length_us(struct qcom_haptics *h,
> + enum qcom_haptics_play_rate rate,
> + u32 data_len)
> +{
> + u32 clk_base = 1000, multiplier = 1, divider = 1;
> +
> + switch (rate) {
> + case PLAY_RATE_T_LRA:
> + clk_base = h->t_lra_us;
> + break;
> + case PLAY_RATE_T_LRA_DIV_2:
> + clk_base = h->t_lra_us;
> + divider = 2;
> + break;
> + case PLAY_RATE_T_LRA_DIV_4:
> + clk_base = h->t_lra_us;
> + divider = 4;
> + break;
> + case PLAY_RATE_T_LRA_DIV_8:
> + clk_base = h->t_lra_us;
> + divider = 8;
> + break;
> + case PLAY_RATE_T_LRA_X_2:
> + clk_base = h->t_lra_us;
> + multiplier = 2;
> + break;
> + case PLAY_RATE_T_LRA_X_4:
> + clk_base = h->t_lra_us;
> + multiplier = 4;
> + break;
> + case PLAY_RATE_T_LRA_X_8:
> + clk_base = h->t_lra_us;
> + multiplier = 8;
> + break;
> + case PLAY_RATE_F_8KHZ:
> + divider = 8;
> + break;
> + case PLAY_RATE_F_16KHZ:
> + divider = 16;
> + break;
> + case PLAY_RATE_F_24KHZ:
> + divider = 24;
> + break;
> + case PLAY_RATE_F_32KHZ:
> + divider = 32;
> + break;
> + case PLAY_RATE_F_44P1KHZ:
> + clk_base *= 10;
> + divider = 441;
> + break;
> + case PLAY_RATE_F_48KHZ:
> + divider = 48;
> + break;
> + default:
> + /* Unexpected rate: fall back to the resonance period. */
> + clk_base = h->t_lra_us;
> + break;
> + }
> +
> + return div_u64((u64)clk_base * data_len * multiplier, divider);
> +}
> +
> +static int haptics_upload_effect(struct input_dev *dev,
> + struct ff_effect *effect,
> + struct ff_effect *old)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> + struct qcom_haptics_effect *priv;
> + int id = effect->id;
> + u32 data_len, level;
> + s8 *fifo;
> +
> + s16 *buf __free(kvfree) = NULL;
> +
> + guard(mutex)(&h->play_lock);
> +
> + if (id < 0 || id >= HAPTICS_MAX_EFFECTS)
> + return -EINVAL;
> +
> + if (id == h->active_effect_id) {
> + dev_err(h->dev, "effect %d is under playing\n", id);
> + return -EBUSY;
> + }
> +
> + priv = &h->effects[id];
> +
> + switch (effect->type) {
> + case FF_CONSTANT:
> + haptics_clear_effect(h, priv);
> + level = effect->u.constant.level <= 0 ? 0 : effect->u.constant.level;
> + priv->amplitude = (u8)mult_frac(level, 255, 0x7FFF);
> + priv->length_us = effect->replay.length * USEC_PER_MSEC;
> + priv->mode = HAPTICS_DIRECT_PLAY;
> + return 0;
> +
> + case FF_PERIODIC:
> + if (effect->u.periodic.waveform != FF_CUSTOM)
> + return -EINVAL;
> + /*
> + * Minimum 3 elements: play-rate code + vmax + at least one sample.
> + * Limit the maximum data length to ~48K so that it can at least
> + * handle ~1s vibration at the fast (48K) play rate.
> + */
> + if (effect->u.periodic.custom_len < 3 ||
> + effect->u.periodic.custom_len > CUSTOM_DATA_MAX_LEN + 2)
> + return -EINVAL;
> +
> + buf = vmemdup_array_user(effect->u.periodic.custom_data,
> + effect->u.periodic.custom_len,
> + sizeof(s16));
> + if (IS_ERR(buf))
> + return PTR_ERR(no_free_ptr(buf));
> +
> + if (buf[CUSTOM_DATA_RATE_IDX] < 0 ||
> + buf[CUSTOM_DATA_RATE_IDX] > PLAY_RATE_MAX ||
> + buf[CUSTOM_DATA_RATE_IDX] == PLAY_RATE_RESERVED)
> + return -EINVAL;
> +
> + data_len = effect->u.periodic.custom_len - CUSTOM_DATA_SAMPLE_START;
> +
> + fifo = kvmalloc(data_len, GFP_KERNEL);
> + if (!fifo)
> + return -ENOMEM;
> +
> + /* Pack: one s8 sample per s16 slot (lower byte) */
> + for (int i = 0; i < data_len; i++)
> + fifo[i] = (s8)buf[CUSTOM_DATA_SAMPLE_START + i];
> +
> + haptics_clear_effect(h, priv);
> +
> + scoped_guard(mutex, &h->fifo_lock) {
> + priv->fifo_data = fifo;
> + priv->data_len = data_len;
> + }
> +
> + priv->play_rate = (u8)buf[CUSTOM_DATA_RATE_IDX];
> + priv->vmax_mv = (u32)clamp_val(buf[CUSTOM_DATA_VMAX_IDX], 0, VMAX_MV_MAX);
> + priv->length_us = haptics_fifo_length_us(h, priv->play_rate, data_len);
> + priv->mode = HAPTICS_FIFO;
> +
> + return 0;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int haptics_playback(struct input_dev *dev, int effect_id, int val)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> + int ret;
> +
> + ret = haptics_enqueue_play_req(h, effect_id, val > 0);
> + if (ret)
> + return ret;
> +
> + if (val > 0)
> + /*
> + * Queue the play. If a duration re-arm is already pending this
> + * is a no-op, so the new play waits for the current effect to
> + * finish before the worker dequeues it.
> + */
> + schedule_delayed_work(&h->play_work, 0);
> + else
> + /*
> + * Run the worker now, cancelling any pending duration re-arm,
> + * so an explicit stop takes effect immediately.
> + */
> + mod_delayed_work(system_percpu_wq, &h->play_work, 0);
> +
> + return 0;
> +}
> +
> +static int haptics_erase(struct input_dev *dev, int effect_id)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> + struct qcom_haptics_effect *priv = &h->effects[effect_id];
> +
> + guard(mutex)(&h->play_lock);
> +
> + haptics_queue_remove_effect(h, effect_id);
> +
> + if (h->active_effect_id == effect_id)
> + haptics_stop_locked(h, false);
> +
> + haptics_clear_effect(h, priv);
> + priv->mode = HAPTICS_MODE_NONE;
> +
> + return 0;
> +}
> +
> +static void haptics_set_gain(struct input_dev *dev, u16 gain)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> +
> + atomic_set(&h->gain, gain);
You need to adjust currently played effects here: new gain should take
effect immediately.
> +}
> +
> +static int qcom_haptics_probe(struct platform_device *pdev)
> +{
> + struct qcom_haptics *h;
> + struct input_dev *input;
> + struct ff_device *ff;
> + u32 regs[2], vmax_uv;
> + int ret, irq;
> +
> + h = devm_kzalloc(&pdev->dev, sizeof(*h), GFP_KERNEL);
> + if (!h)
> + return -ENOMEM;
> +
> + h->dev = &pdev->dev;
> +
> + h->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!h->regmap)
> + return dev_err_probe(h->dev, -ENODEV, "no regmap from parent\n");
> +
> + ret = device_property_read_u32_array(h->dev, "reg", regs, ARRAY_SIZE(regs));
> + if (ret)
> + return dev_err_probe(h->dev, ret, "failed to read 'reg' property\n");
> +
> + h->cfg_base = regs[0];
> + h->ptn_base = regs[1];
> +
> + ret = device_property_read_u32(h->dev, "qcom,lra-period-us", &h->t_lra_us);
> + if (ret)
> + return dev_err_probe(h->dev, ret, "missing qcom,lra-period-us\n");
> +
> + h->t_lra_us = clamp(h->t_lra_us, (u32)TLRA_STEP_US, (u32)TLRA_US_MAX);
> +
> + ret = device_property_read_u32(h->dev, "qcom,vmax-microvolt", &vmax_uv);
> + if (ret)
> + return dev_err_probe(h->dev, ret, "missing qcom,vmax-microvolt\n");
> +
> + h->vmax_mv = clamp(vmax_uv / 1000, (u32)VMAX_STEP_MV, (u32)VMAX_MV_MAX);
> +
> + ret = haptics_config_lra_period(h);
> + if (ret)
> + return ret;
> +
> + ret = haptics_configure_autores(h);
> + if (ret)
> + return ret;
> +
> + ret = haptics_set_vmax(h, h->vmax_mv);
> + if (ret)
> + return ret;
> +
> + ret = haptics_configure_fifo_mmap(h);
> + if (ret)
> + return ret;
> +
> + mutex_init(&h->play_lock);
> + mutex_init(&h->fifo_lock);
> + spin_lock_init(&h->play_queue_lock);
> + INIT_LIST_HEAD(&h->play_queue);
> + INIT_DELAYED_WORK(&h->play_work, haptics_play_work);
> + atomic_set(&h->gain, 0xFFFF);
> + h->active_effect_id = -1;
> +
> + irq = platform_get_irq_byname(pdev, "fifo-empty");
> + if (irq < 0)
> + return dev_err_probe(h->dev, irq, "failed to get fifo-empty IRQ\n");
> +
> + ret = devm_request_threaded_irq(h->dev, irq, NULL,
> + haptics_fifo_empty_irq,
> + IRQF_ONESHOT | IRQF_NO_AUTOEN,
> + "qcom-haptics-fifo-empty", h);
> + if (ret)
> + return dev_err_probe(h->dev, ret, "failed to request fifo-empty IRQ\n");
> +
> + h->fifo_empty_irq = irq;
> + platform_set_drvdata(pdev, h);
> +
> + pm_runtime_use_autosuspend(h->dev);
> + pm_runtime_set_autosuspend_delay(h->dev, HAPTICS_AUTOSUSPEND_MS);
> + ret = devm_pm_runtime_enable(h->dev);
> + if (ret)
> + return dev_err_probe(h->dev, ret, "enable runtime PM failed\n");
> +
> + input = devm_input_allocate_device(h->dev);
> + if (!input)
> + return -ENOMEM;
> +
> + input->name = "qcom-spmi-haptics";
Set bus type? BUS_HOST?
> + input_set_drvdata(input, h);
> + h->input = input;
> +
> + input_set_capability(input, EV_FF, FF_CONSTANT);
> + input_set_capability(input, EV_FF, FF_PERIODIC);
> + input_set_capability(input, EV_FF, FF_CUSTOM);
> + input_set_capability(input, EV_FF, FF_GAIN);
> +
> + ret = input_ff_create(input, HAPTICS_MAX_EFFECTS);
> + if (ret)
> + return ret;
> +
> + ff = input->ff;
> + ff->upload = haptics_upload_effect;
> + ff->playback = haptics_playback;
> + ff->erase = haptics_erase;
> + ff->set_gain = haptics_set_gain;
> +
> + ret = input_register_device(input);
> + if (ret) {
> + input_ff_destroy(input);
This is harmless but not needed: FF portion will be freed automatically.
> + return dev_err_probe(h->dev, ret, "failed to register input device\n");
> + }
> +
> + return 0;
> +}
> +
> +static void qcom_haptics_remove(struct platform_device *pdev)
> +{
> + struct qcom_haptics *h = platform_get_drvdata(pdev);
> +
> + /*
> + * Unregister the input device explicitly at the beginning
> + * to avoid the input device being used after the resources
> + * are freed.
> + */
> + input_unregister_device(h->input);
> + disable_delayed_work_sync(&h->play_work);
> + scoped_guard(mutex, &h->play_lock) {
> + haptics_queue_flush(h);
> + haptics_stop_locked(h, true);
> + }
> +
> + haptics_enable_module(h, false);
> +
> + scoped_guard(mutex, &h->fifo_lock) {
> + for (int i = 0; i < HAPTICS_MAX_EFFECTS; i++) {
> + kvfree(h->effects[i].fifo_data);
> + h->effects[i].fifo_data = NULL;
> + }
> + }
Most of this is better suited to close() method of input device.
> +}
> +
> +static int qcom_haptics_runtime_suspend(struct device *dev)
> +{
> + struct qcom_haptics *h = dev_get_drvdata(dev);
> +
> + return haptics_enable_module(h, false);
> +}
> +
> +static int qcom_haptics_runtime_resume(struct device *dev)
> +{
> + struct qcom_haptics *h = dev_get_drvdata(dev);
> +
> + return haptics_enable_module(h, true);
> +}
> +
> +static int qcom_haptics_suspend(struct device *dev)
> +{
> + struct qcom_haptics *h = dev_get_drvdata(dev);
> + int ret;
> +
> + disable_delayed_work_sync(&h->play_work);
> + scoped_guard(mutex, &h->play_lock) {
> + haptics_queue_flush(h);
> + haptics_stop_locked(h, true);
> + }
> +
> + ret = pm_runtime_force_suspend(dev);
> + if (ret)
> + enable_delayed_work(&h->play_work);
> +
> + return ret;
> +}
> +
> +static int qcom_haptics_resume(struct device *dev)
> +{
> + struct qcom_haptics *h = dev_get_drvdata(dev);
> +
> + enable_delayed_work(&h->play_work);
> +
> + return pm_runtime_force_resume(dev);
> +}
> +
> +static const struct dev_pm_ops qcom_haptics_pm_ops = {
> + SYSTEM_SLEEP_PM_OPS(qcom_haptics_suspend, qcom_haptics_resume)
> + RUNTIME_PM_OPS(qcom_haptics_runtime_suspend, qcom_haptics_runtime_resume,
> + NULL)
> +};
> +
> +static const struct of_device_id qcom_haptics_of_match[] = {
> + { .compatible = "qcom,spmi-haptics" },
> + {}
{ }
> +};
> +MODULE_DEVICE_TABLE(of, qcom_haptics_of_match);
> +
> +static struct platform_driver qcom_haptics_driver = {
> + .probe = qcom_haptics_probe,
> + .remove = qcom_haptics_remove,
> + .driver = {
> + .name = "qcom-spmi-haptics",
> + .of_match_table = qcom_haptics_of_match,
> + .pm = pm_ptr(&qcom_haptics_pm_ops),
> + },
> +};
> +module_platform_driver(qcom_haptics_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm SPMI PMIC Haptics driver");
> +MODULE_LICENSE("GPL");
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: input: focaltech,ft8112: Add focaltech,ft3d81 compatible
From: Dmitry Torokhov @ 2026-07-17 16:54 UTC (permalink / raw)
To: Pradyot Kumar Nayak
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Daniel Peng,
Bjorn Andersson, Konrad Dybcio, Abel Vesa, Dmitry Baryshkov,
linux-arm-msm, linux-input, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski
In-Reply-To: <20260717-add_focaltech_ft3d81_touchscreen_support-v4-1-5dd091e25801@oss.qualcomm.com>
On Fri, Jul 17, 2026 at 05:28:34PM +0530, Pradyot Kumar Nayak wrote:
> The Focaltech ft3d81 is fully compatible with the ft8112 i.e.
> it uses the same I2C-HID protocol and the same power-on/reset sequencing,
> DT nodes for boards carrying an ft3d81,can therefore bind to the existing
> ft8112 driver without any additional changes.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox