* [PATCH v5 1/3] drm/bridge: ti-sn65dsi83: Simplify error condition logic
From: Wojciech Dubowik @ 2026-07-17 11:49 UTC (permalink / raw)
To: linux-kernel
Cc: Wojciech Dubowik, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Marek Vasut, dri-devel, devicetree
In-Reply-To: <20260717115002.308035-1-wojciech.dubowik@mt.com>
From: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
Simplify error condition logic in lvds device tree property parsing. It
makes it easier to add extra properties without handling complex goto
entries.
Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 43 ++++++++-------------------
1 file changed, 13 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 42b451432bbb..f8a786953526 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -825,48 +825,35 @@ static int sn65dsi83_select_lvds_vod_swing(struct device *dev,
static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
{
struct device *dev = ctx->dev;
- struct device_node *endpoint;
- int endpoint_reg;
+ int endpoint_reg = (channel == CHANNEL_A) ? 2 : 3;
+ struct device_node *endpoint __free(device_node) =
+ of_graph_get_endpoint_by_regs(dev->of_node, endpoint_reg, -1);
/* Set so the property can be freely selected if not defined */
u32 lvds_vod_swing_data[2] = { 0, 1000000 };
u32 lvds_vod_swing_clk[2] = { 0, 1000000 };
/* Set default near end terminataion to 200 Ohm */
u32 lvds_term = 200;
int lvds_vod_swing_conf;
- int ret = 0;
int ret_data;
int ret_clock;
- if (channel == CHANNEL_A)
- endpoint_reg = 2;
- else
- endpoint_reg = 3;
-
- endpoint = of_graph_get_endpoint_by_regs(dev->of_node, endpoint_reg, -1);
-
of_property_read_u32(endpoint, "ti,lvds-termination-ohms", &lvds_term);
if (lvds_term == 100)
ctx->lvds_term_conf[channel] = OHM_100;
else if (lvds_term == 200)
ctx->lvds_term_conf[channel] = OHM_200;
- else {
- ret = -EINVAL;
- goto exit;
- }
+ else
+ return -EINVAL;
ret_data = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-data-microvolt",
lvds_vod_swing_data, ARRAY_SIZE(lvds_vod_swing_data));
- if (ret_data != 0 && ret_data != -EINVAL) {
- ret = ret_data;
- goto exit;
- }
+ if (ret_data != 0 && ret_data != -EINVAL)
+ return ret_data;
ret_clock = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-clock-microvolt",
lvds_vod_swing_clk, ARRAY_SIZE(lvds_vod_swing_clk));
- if (ret_clock != 0 && ret_clock != -EINVAL) {
- ret = ret_clock;
- goto exit;
- }
+ if (ret_clock != 0 && ret_clock != -EINVAL)
+ return ret_clock;
/* Use default value if both properties are NOT defined. */
if (ret_data == -EINVAL && ret_clock == -EINVAL)
@@ -876,17 +863,13 @@ static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
if (!ret_data || !ret_clock) {
lvds_vod_swing_conf = sn65dsi83_select_lvds_vod_swing(dev, lvds_vod_swing_data,
lvds_vod_swing_clk, ctx->lvds_term_conf[channel]);
- if (lvds_vod_swing_conf < 0) {
- ret = lvds_vod_swing_conf;
- goto exit;
- }
+ if (lvds_vod_swing_conf < 0)
+ return lvds_vod_swing_conf;
}
ctx->lvds_vod_swing_conf[channel] = lvds_vod_swing_conf;
- ret = 0;
-exit:
- of_node_put(endpoint);
- return ret;
+
+ return 0;
}
static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
--
2.47.3
^ permalink raw reply related
* [PATCH 2/2] arm64: dts: qcom: milos: Add CPU-to-DDR BWMON support
From: Luca Weiss @ 2026-07-17 11:45 UTC (permalink / raw)
To: Krzysztof Kozlowski, Georgi Djakov, Rob Herring, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-pm,
devicetree, linux-kernel, Luca Weiss
In-Reply-To: <20260717-milos-cpu-bwmon-v1-0-dfe0bac9898c@fairphone.com>
Add a node for the CPU-to-DDR BWMON on the Milos SoC and the
corresponding OPP table.
Currently, the table is for DDR5, the values for DDR4 are put into a
TODO comment, until the kernel can differentiate between DDR variants.
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
arch/arm64/boot/dts/qcom/milos.dtsi | 49 +++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/milos.dtsi b/arch/arm64/boot/dts/qcom/milos.dtsi
index e91c0cf5e093..9b79e5bdfbfb 100644
--- a/arch/arm64/boot/dts/qcom/milos.dtsi
+++ b/arch/arm64/boot/dts/qcom/milos.dtsi
@@ -2940,6 +2940,55 @@ cpufreq_hw: cpufreq@17d91000 {
#clock-cells = <1>;
};
+ pmu@24091000 {
+ compatible = "qcom,milos-cpu-bwmon", "qcom,sc7280-llcc-bwmon";
+ reg = <0x0 0x24091000 0x0 0x1000>;
+
+ interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH 0>;
+
+ interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ACTIVE_ONLY>;
+
+ operating-points-v2 = <&llcc_bwmon_opp_table>;
+
+ llcc_bwmon_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-0 {
+ opp-peak-kBps = <2086000>;
+ };
+
+ opp-1 {
+ opp-peak-kBps = <2929000>;
+ };
+
+ opp-2 {
+ /* TODO: ddr4 = 3879000 */
+ opp-peak-kBps = <5931000>;
+ };
+
+ opp-3 {
+ /* TODO: ddr4 = 5163000 */
+ opp-peak-kBps = <6515000>;
+ };
+
+ opp-4 {
+ /* TODO: ddr4 = 5931000 */
+ opp-peak-kBps = <7980000>;
+ };
+
+ opp-5 {
+ /* TODO: ddr4 = 6515000 */
+ opp-peak-kBps = <10437000>;
+ };
+
+ opp-6 {
+ /* TODO: ddr4 = 7980000 */
+ opp-peak-kBps = <12191000>;
+ };
+ };
+ };
+
gem_noc: interconnect@24100000 {
compatible = "qcom,milos-gem-noc";
reg = <0x0 0x24100000 0x0 0xff080>;
--
2.55.0
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: interconnect: qcom-bwmon: Add Milos cpu-bwmon compatible
From: Luca Weiss @ 2026-07-17 11:45 UTC (permalink / raw)
To: Krzysztof Kozlowski, Georgi Djakov, Rob Herring, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-pm,
devicetree, linux-kernel, Luca Weiss
In-Reply-To: <20260717-milos-cpu-bwmon-v1-0-dfe0bac9898c@fairphone.com>
Add the compatible string for the CPU-to-DDR bandwidth monitor on the
Milos SoC, which is BWMONv5.
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml b/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
index c0a5e925ff95..4e33c75d9243 100644
--- a/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
+++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
@@ -49,6 +49,7 @@ properties:
- enum:
- qcom,eliza-llcc-bwmon
- qcom,hawi-llcc-bwmon
+ - qcom,milos-cpu-bwmon
- qcom,qcs615-llcc-bwmon
- qcom,qcs8300-llcc-bwmon
- qcom,sa8775p-llcc-bwmon
--
2.55.0
^ permalink raw reply related
* [PATCH 0/2] Add Milos CPU-to-DDR BWMON support
From: Luca Weiss @ 2026-07-17 11:45 UTC (permalink / raw)
To: Krzysztof Kozlowski, Georgi Djakov, Rob Herring, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-pm,
devicetree, linux-kernel, Luca Weiss
Add support for the bandwidth monitoring block on Milos which lets the
driver vote on interconnect bandwidth based on monitored throughput.
Similar to the CPU OPP table series[0], this would benefit from driver
support that can differentiate between DDR4 and DDR5 RAM since different
votes would be correct.
[0] https://lore.kernel.org/linux-arm-msm/20260710-milos-cpu-opp-v1-0-ae7f4b09bc77@fairphone.com/
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Luca Weiss (2):
dt-bindings: interconnect: qcom-bwmon: Add Milos cpu-bwmon compatible
arm64: dts: qcom: milos: Add CPU-to-DDR BWMON support
.../bindings/interconnect/qcom,msm8998-bwmon.yaml | 1 +
arch/arm64/boot/dts/qcom/milos.dtsi | 49 ++++++++++++++++++++++
2 files changed, 50 insertions(+)
---
base-commit: 19827e7930a5511c68e12aac6e4522f3661a1709
change-id: 20260717-milos-cpu-bwmon-fce32974efb1
Best regards,
--
Luca Weiss <luca.weiss@fairphone.com>
^ permalink raw reply
* Re: [PATCH v3 3/6] arm64: dts: qcom: kodiak: Add GEM_NOC interconnect for adreno SMMU
From: Bibek Kumar Patro @ 2026-07-17 11:40 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Dmitry Baryshkov, Will Deacon, Robin Murphy, Joerg Roedel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio,
Rob Clark, linux-arm-kernel, iommu, devicetree, linux-kernel,
linux-arm-msm
In-Reply-To: <ak5qkRKk2eHHLYmo@baldur>
On 7/8/2026 8:56 PM, Bjorn Andersson wrote:
> On Wed, Jul 08, 2026 at 03:31:07PM +0530, Bibek Kumar Patro wrote:
>>
>>
>> On 7/7/2026 10:14 PM, Dmitry Baryshkov wrote:
>>> On Tue, Jul 07, 2026 at 06:37:24PM +0530, Bibek Kumar Patro wrote:
>>>>
>>>>
>>>> On 7/6/2026 11:12 PM, Dmitry Baryshkov wrote:
>>>>> On Mon, Jul 06, 2026 at 10:26:36PM +0530, Bibek Kumar Patro wrote:
>>>>>> On Kodiak platforms, the Adreno SMMU requires a bandwidth vote on
>>>>>> the GEM_NOC path (MASTER_GPU_TCU -> SLAVE_EBI1) before its registers
>>>>>> are accessible. Without this vote, the SMMU may become unreachable,
>>>>>> leading to intermittent probe failures and runtime issues.
>>>>>>
>>>>>> Add the required interconnect to ensure reliable register access.
>>>>>>
>>>>>> Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
>>>>>> ---
>>>>>> arch/arm64/boot/dts/qcom/kodiak.dtsi | 2 ++
>>>>>> 1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>>>> index 90e50c245c0c..721526f023dd 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>>>> @@ -3389,6 +3389,8 @@ adreno_smmu: iommu@3da0000 {
>>>>>> power-domains = <&gpucc GPU_CC_CX_GDSC>;
>>>>>> dma-coherent;
>>>>>> + interconnects = <&gem_noc MASTER_GPU_TCU QCOM_ICC_TAG_ALWAYS
>>>>>> + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>>>>>
>>>>> Why is it ALWAYS? Would it be better to declare it as ACTIVE_ONLY and
>>>>
>>>> Added QCOM_ICC_TAG_ALWAYS, to hold the vote in SLEEP bucket as well
>>>> preventing gem_noc going to sleep when icc_set_bw is being called.
>>>
>>> _why_?
>>>
>>
>> Now when i think of where we implemented the hooks, ALWAYS is not needed. We
>> should keep it ACTIVE_ONLY since we are already voting it
>> back in resume pah. I'll fix it in next revision.
>>
>
> The path is between the SMMU and DDR, so clearly it has to do with the
> TCU's ability to fetch things from DDR.
>
Yes as you rightly mentioned, this series is helping to resolve the race
in TCU DDR clink
> ACTIVE_ONLY states that the vote should only be applied when the
> application subsystem (CPUSS) is not in low-power mode (CPU is active).
>
> So, are you saying that in all cases that cpuidle might select such
> low-power state the GPU TCU is "stopped", and does not need to perform
> any memory transactions?
>
I rechecked the architectural block diagram, GPU TCU is
independent of the CPUSS, unlike system TCU.
This means CPUSS can be powered down while the GPU TCU remains voted up.
While this series was intended to avoid a DDR voting race between the
GPU TCU and its clients, using ACTIVE_ONLY could have possibly
introduced a race between CPUSS and the GPU TCU itself.
Thanks for catching this in my reply Bjorn.
ACTIVE_ONLY would have been suitable for system TCU (apps_smmu)
for which the interconnect field is anyways not needed, but
unsuitable for GPU TCU.
>
> To me these two are orthogonal and TAG_ALWAYS is logically correct, but
> perhaps there's some practical dependency that I'm missing?
>
We can ignore my acknowledgment [1] on previous response, Dmitry.
ALWAYS TAG is correct here instead of ACTIVE_ONLY in the interconnect
field for adreno SMMU.
[1]:
https://lore.kernel.org/all/4e5a3d82-b946-4490-885a-ea7061939c32@oss.qualcomm.com/#:~:text=Yes%2C%20I%20think%20we%20should%20go%20with%20the%20ACTIVE_ONLY%20flag
Thanks & regards,
Bibek
> Regards,
> Bjorn
>
>>>>
>>>>> then drop the extra suspend/resume play?
>>>>
>>>> Not sure if I understood it correctly.
>>>> Did you mean the extra suspend/resume play in arm_smmu_runtime_suspend
>>>> or in arm_smmu_device_shutdown() path?
>>>
>>> runtime_suspend/resume. I might be incorrect here, but I think it is
>>> exactly what you need.
>>>
>>
>> Yes, I think we should go with the ACTIVE_ONLY flag and we would need
>> this voting in suspend resume path (For the same reason mentioned in commit
>> description and cover letter of the series).
>>
>> ALWAYS would avoid the need for this handling, it would keep the vote active
>> unnecessarily, but still that would have been nondeterministic.
>>
>> Thanks & regards,
>> Bibek
>>
>>>>
>>>> Thanks & regards,
>>>> Bibek
>>>>
>>>>>
>>>>>> };
>>>>>> gfx_0_tbu: tbu@3dd9000 {
>>>>>>
>>>>>> --
>>>>>> 2.34.1
>>>>>>
>>>>>
>>>>
>>>
>>
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] net: phy: add DAPU Telecom DAP8210R(I) Gigabit Ethernet PHY driver
From: sashiko-bot @ 2026-07-17 11:38 UTC (permalink / raw)
To: Artem Shimko; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260716113805.593215-3-a.shimko.dev@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unconfigured RGMII delays are not explicitly disabled, leaking hardware state.
- [High] Software reset is triggered before RGMII settings are written.
- [Medium] Invalid RGMII delay values in the Device Tree are silently ignored instead of failing probe.
- [Low] The Kconfig help text claims support for an 'internal packet generator' that is not implemented.
- [Low] The <linux/delay.h> header is not explicitly included despite using fsleep().
--
--- Patch [2]: [PATCH net-next v2 2/2] net: phy: add DAPU Telecom DAP8210R(I) Gigabit Ethernet PHY driver ---
commit a12532ee64a58b8261da15777e57b6a0bb7a0bc4
Author: Artem Shimko <a.shimko.dev@gmail.com>
net: phy: add DAPU Telecom DAP8210R(I) Gigabit Ethernet PHY driver
This commit adds a new PHY driver for the DAPU Telecom DAP8211R(I) Gigabit
Ethernet PHY. It implements extended register access and provides device
tree support for RGMII delay configuration and TX clock phase shifting.
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 099f25dceabbc..4576f707ac944 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
[ ... ]
> @@ -237,6 +237,16 @@ config DAVICOM_PHY
> help
> Currently supports dm9161e and dm9131
>
> +config DAP8211R_PHY
> + tristate "DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY"
> + depends on OF
> + help
> + Support for the DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY.
> + This PHY is designed for enterprise and industrial networking
> + applications, supporting 10/100/1000 Mbps operation.
> + RGMII with: configurable TX/RX clock delays, optional flag to enable
> + 180-degree TX clock phase shift and internal packet generator.
[Severity: Low]
Is this help text accurate regarding the internal packet generator?
The description mentions an internal packet generator, but there doesn't
appear to be any functionality for it implemented in the driver itself.
> diff --git a/drivers/net/phy/dap8211r.c b/drivers/net/phy/dap8211r.c
> new file mode 100644
> index 0000000000000..e1e6a322ef0c4
> --- /dev/null
> +++ b/drivers/net/phy/dap8211r.c
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL
> +/*
> + * Driver for the DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY.
[ ... ]
> +#include <linux/bitfield.h>
> +#include <linux/errno.h>
> +#include <linux/ethtool.h>
[Severity: Low]
Does this file need to include <linux/delay.h>?
The code uses fsleep() in dap8211r_config_init(), but the delay header
is not explicitly included. This might cause build failures on some
architectures or configs.
[ ... ]
> +static int dap8211r_delay_ps_to_sel(u32 ps)
> +{
> + for (int i = 0; i < DAP8211R_DELAY_COUNT; i++)
> + if (ps == delay_config[i].ps)
> + return delay_config[i].sel;
> +
> + return -EINVAL;
> +}
[ ... ]
> +static int dap8211r_get_rgmii_delay(struct phy_device *phydev, const char *prop_name, bool is_id)
> +{
> + struct device_node *np = phydev->mdio.dev.of_node;
> + u32 ps = 0;
> + int ret;
> +
> + ret = of_property_read_u32(np, prop_name, &ps);
> + if (ret == -EINVAL)
> + return (is_id) ? DAP8211R_DEFAULT_DELAY_SEL : ret;
> + if (ret < 0)
> + return ret;
> +
> + return dap8211r_delay_ps_to_sel(ps);
> +}
[ ... ]
> +static int dap8211r_config_init(struct phy_device *phydev)
> +{
> + struct device_node *phydev_node = phydev->mdio.dev.of_node;
> + u16 mask = 0, set = 0;
> + int ret, retries = 10;
> +
> + switch (phydev->interface) {
> + case PHY_INTERFACE_MODE_RGMII:
> + ret = dap8211r_get_rgmii_delay(phydev, "rx-internal-delay-ps", false);
> + if (ret >= 0) {
> + set = FIELD_PREP(DAP8211R_RGMII_RX_DEL_MASK, ret);
> + mask = DAP8211R_RGMII_RX_DEL_MASK;
> + } else if ((ret < 0) && (ret != -EINVAL)) {
> + return ret;
> + }
[Severity: Medium]
Does this logic unintentionally skip invalid delay values in the device tree?
Because dap8211r_get_rgmii_delay() returns -EINVAL when a property is
entirely missing, and dap8211r_delay_ps_to_sel() also returns -EINVAL for
an invalid out-of-bounds value (like rx-internal-delay-ps = <200>), the
check here treats both conditions exactly the same.
This means if an invalid delay is provided in the device tree, it will be
silently ignored instead of failing probe.
[Severity: High]
Will this leave stale hardware default delays enabled if properties are missing?
In PHY_INTERFACE_MODE_RGMII, if the delay properties are not provided,
the mask bits remain empty. The driver doesn't explicitly clear the
DAP8211R_RGMII_RX_DEL_MASK or DAP8211R_RGMII_TX_DEL_MASK bits in the mask.
If the bootloader or hardware defaults left these delays active, they will
not be disabled, violating the requested PHY_INTERFACE_MODE_RGMII mode.
> +
> + ret = dap8211r_get_rgmii_delay(phydev, "tx-internal-delay-ps", false);
> + if (ret >= 0) {
> + set |= FIELD_PREP(DAP8211R_RGMII_TX_DEL_MASK, ret);
> + mask |= DAP8211R_RGMII_TX_DEL_MASK;
> + } else if ((ret < 0) && (ret != -EINVAL)) {
> + return ret;
> + }
> + break;
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + ret = dap8211r_get_rgmii_delay(phydev, "rx-internal-delay-ps", true);
> + if (ret < 0)
> + return ret;
> +
> + set = FIELD_PREP(DAP8211R_RGMII_RX_DEL_MASK, ret);
> + mask = DAP8211R_RGMII_RX_DEL_MASK;
> + break;
[Severity: High]
Similarly, in PHY_INTERFACE_MODE_RGMII_RXID, the TX delay mask is never added
to the 'mask' variable. Could this leave the TX delay active if it was
previously configured by the bootloader?
[ ... ]
> + if (of_property_read_bool(phydev_node, "dapu,tx-inverted-clk"))
> + set |= DAP8211R_RGMII_CLK_INVERT;
> +
> + mask |= DAP8211R_RGMII_CLK_INVERT;
> +
> + ret = dap8211r_modify_ext(phydev, DAP8211R_PHY_CON, DAP8211R_PHY_SW_RST, 0);
> + if (ret)
> + return ret;
> +
> + /* Wait for reset self-clear */
> + do {
> + fsleep(20);
> + ret = dap8211r_read_ext(phydev, DAP8211R_PHY_CON);
> + if (ret < 0)
> + return ret;
> + } while (!(ret & DAP8211R_PHY_SW_RST) && --retries);
> +
> + if (!retries)
> + return -ETIMEDOUT;
> +
> + ret = dap8211r_modify_ext(phydev, DAP8211R_RGMII_CON, mask, set);
> + if (ret)
> + return ret;
[Severity: High]
Are the RGMII settings applied correctly here?
The comment earlier in dap8211r_config_init() mentions "Software reset to
apply settings", but the software reset is executed and waited upon before
writing to DAP8211R_RGMII_CON.
Because the software reset happens before the new delay configuration is written,
does this mean the newly programmed RGMII delays and clock inversions will not
be latched into the PHY's datapath?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716113805.593215-1-a.shimko.dev@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] dt-bindings: net: add DAPU Telecom DAP8211R(I) PHY binding
From: sashiko-bot @ 2026-07-17 11:38 UTC (permalink / raw)
To: Artem Shimko; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260716113805.593215-2-a.shimko.dev@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The 'dapu' vendor prefix used in 'dapu,tx-inverted-clk' is not documented in vendor-prefixes.yaml.
--
--- Patch [1]: [PATCH net-next v2 1/2] dt-bindings: net: add DAPU Telecom DAP8211R(I) PHY binding ---
commit 05e0cd0742ef4792c4b9865b36c3662e45293902
Author: Artem Shimko <a.shimko.dev@gmail.com>
dt-bindings: net: add DAPU Telecom DAP8211R(I) PHY binding
This commit adds device tree binding documentation for the DAPU Telecom
DAP8211R(I) Gigabit Ethernet PHY, including properties for configuring
TX and RX clock delays and a vendor-specific flag for clock inversion.
> diff --git a/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml b/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
[ ... ]
> + tx-internal-delay-ps:
> + description:
> + RGMII TX clock delay in picoseconds (0 to maximum).
> + multipleOf: 150
> + maximum: 2250
> + default: 1950
> +
> + dapu,tx-inverted-clk:
[Severity: Low]
Is the dapu vendor prefix documented in vendor-prefixes.yaml?
The dt-schema tool strictly validates all vendor prefixes against the
registry. Because it appears to be missing from
Documentation/devicetree/bindings/vendor-prefixes.yaml, running
make dt_binding_check will result in a schema validation error for an
undocumented vendor prefix.
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + If present, the RGMII TX clock to the MAC is inverted (180 degree
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716113805.593215-1-a.shimko.dev@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] dt-bindings: i3c: dw: Document missing optional core reset
From: Krzysztof Kozlowski @ 2026-07-17 11:33 UTC (permalink / raw)
To: Shubham Patil, alexandre.belloni, Frank.Li, robh, krzk+dt,
conor+dt
Cc: linux-i3c, devicetree, linux-kernel, radhey.shyam.pandey,
michal.simek
In-Reply-To: <20260717105829.567522-1-shubhamsanjay.patil@amd.com>
On 17/07/2026 12:58, Shubham Patil wrote:
> The DesignWare I3C master IP supports a core reset input, but the
> binding is missing the corresponding reset description.
>
> Document the reset property and name in the binding. Keep the reset
> optional because the reset line is integration-specific..
>
> Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
> ---
> Note:
> The DesignWare I3C driver support was added by
> 1dd728f5d4d4 ("i3c: master: Add driver for Synopsys DesignWare IP").
>
> The original Synopsys DesignWare I3C binding was added by
> 116e246a8bcd ("dt-binding: i3c: Document Synopsys DesignWare I3C"),
> but it did not document the optional core reset.
> ---
> .../devicetree/bindings/i3c/snps,dw-i3c-master.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> index e803457d3f55..822faf8d221b 100644
> --- a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> +++ b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> @@ -38,6 +38,12 @@ properties:
> interrupts:
> maxItems: 1
>
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + const: core_rst
Drop the names, pretty redundant, and fix the driver. Sneaking ABI is a
poor excuse, IMO, to keep discouraged names.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: Add Xiaomi 11 Lite 5G NE
From: Konrad Dybcio @ 2026-07-17 11:32 UTC (permalink / raw)
To: Oleksii Onchul, Bjorn Andersson, Konrad Dybcio
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
devicetree, linux-kernel
In-Reply-To: <20260714125806.310708-3-oleksiionchul@gmail.com>
On 7/14/26 2:58 PM, Oleksii Onchul wrote:
> Add initial support for the Xiaomi 11 Lite 5G NE, codenamed lisa.
>
> The supported hardware includes regulators, UFS, USB, GPU, remote
> processors, Wi-Fi, Bluetooth, touchscreen, flash LED, thermal sensors
> and the hardware buttons.
>
> Signed-off-by: Oleksii Onchul <oleksiionchul@gmail.com>
> ---
[...]
> +&i2c1 {
> + clock-frequency = <100000>;
> + status = "okay";
nit: let's keep a \n before 'status', file-wide
[...]
> +&spi13 {
> + status = "okay";
> +
> + touchscreen@0 {
> + compatible = "goodix,gt9897";
> + reg = <0>;
> + spi-max-frequency = <6000000>;
> +
> + interrupt-parent = <&tlmm>;
> + interrupts = <81 IRQ_TYPE_LEVEL_LOW>;
interrupts-extended = <&tlmm 81 IRQ..
With that:
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [RFC PATCH 4/9] iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active
From: Chaoyi Chen @ 2026-07-17 10:15 UTC (permalink / raw)
To: Jiaxing Hu, tomeu, heiko, robh, krzk+dt, conor+dt, joro, will,
robin.murphy, ulfh, p.zabel, ogabbay
Cc: dri-devel, linux-rockchip, iommu, linux-pm, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260717085220.3212274-5-gahing@gahingwoo.com>
Hello Jiaxing,
On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> Boot firmware can leave a PAGE_FAULT_ACTIVE condition in an IOMMU bank
> before the driver has configured paging. Such a bank shows:
>
I'm not sure what's happening here. Does this issue still occur when
you reset the MMU?
> PAGE_FAULT_ACTIVE=1 STALL_ACTIVE=0 IDLE=1
>
> rk_iommu_is_stall_active() sees STALL_ACTIVE=0 and reports the whole
> IOMMU as "not stalled", so any subsequent readx_poll_timeout() loop
> waiting for the stall to complete never sees a passing result even
> after the other banks have correctly entered stall mode.
>
> Detect the orphaned-fault pattern (PAGE_FAULT_ACTIVE & !STALL_ACTIVE &
> IDLE) and skip those banks in the stall check. They have no in-flight
> transaction, so they are already quiescent.
>
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
> drivers/iommu/rockchip-iommu.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 0013cf196..f42ffcafb 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -378,9 +378,23 @@ static bool rk_iommu_is_stall_active(struct rk_iommu *iommu)
> bool active = true;
> int i;
>
> - for (i = 0; i < iommu->num_mmu; i++)
> - active &= !!(rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
> - RK_MMU_STATUS_STALL_ACTIVE);
> + for (i = 0; i < iommu->num_mmu; i++) {
> + u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
> +
> + /*
> + * A bank stuck with PAGE_FAULT_ACTIVE but without STALL_ACTIVE
> + * and with IDLE set has an orphaned fault left by firmware before
> + * paging was configured. It cannot enter stall mode but has no
> + * transaction in flight, so it is already quiescent. Skip it
> + * rather than treating it as "not stalled."
> + */
> + if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
> + !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
> + (status & RK_MMU_STATUS_IDLE))
> + continue;
> +
> + active &= !!(status & RK_MMU_STATUS_STALL_ACTIVE);
> + }
>
> return active;
> }
--
Best,
Chaoyi
^ permalink raw reply
* Re: [PATCH v2 6/6] arm64: dts: qcom: qcs6490-rubikpi3: Add audio support
From: Konrad Dybcio @ 2026-07-17 11:30 UTC (permalink / raw)
To: Hongyang Zhao, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Daniel Drake, Katsuhiro Suzuki,
Matteo Martelli, Binbin Zhou, Srinivas Kandagatla,
Jaroslav Kysela, Takashi Iwai, Bjorn Andersson, Konrad Dybcio
Cc: Mohammad Rafi Shaik, Neil Armstrong, Dmitry Baryshkov,
Roger Shimizu, linux-sound, devicetree, linux-kernel,
linux-arm-msm
In-Reply-To: <20260715-rubikpi-next-20260605-v2-6-67601d7a8581@thundersoft.com>
On 7/15/26 4:02 PM, Hongyang Zhao wrote:
> RubikPi 3 uses the AudioReach firmware path for its external audio
> interfaces. The primary MI2S interface connects to an ES8316 codec for
> headset playback and capture, while the quaternary MI2S interface feeds
> the LT9611 HDMI bridge.
[...]
> + vreg_es8316_3v3: regulator-es8316-3v3 {
> + compatible = "regulator-fixed";
> +
> + regulator-name = "vreg_es8316_3v3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + gpio = <&tlmm 117 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-0 = <&es8316_power_on>;
> + pinctrl-names = "default";
> +
> + regulator-always-on;
> + regulator-boot-on;
Does it need to be always-on? Is it really on at boot time?
[...]
> + mi2s-playback-dai-link {
mi2s-'c'apture < mi2s-'p'layback
Konrad
^ permalink raw reply
* Re: [PATCH v4 04/11] arm64: dts: qcom: x1e80100-crd: Add pm8010 CRD pmic,id=m regulators
From: Konrad Dybcio @ 2026-07-17 11:21 UTC (permalink / raw)
To: Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260708-x1e-camss-csi2-phy-dtsi-v4-4-572348ad1b2a@linaro.org>
On 7/8/26 2:44 AM, Bryan O'Donoghue wrote:
> Add pmic,id = m rpmh to regulator definitions. This regulator set provides
> vreg_l3m_1p8 the regulator for the ov08x40 RGB sensor on the CRD.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v28 4/7] firmware: imx: device context dedicated to priv
From: sashiko-bot @ 2026-07-17 11:13 UTC (permalink / raw)
To: pankaj.gupta; +Cc: Frank.Li, robh, conor+dt, devicetree, imx
In-Reply-To: <20260717-imx-se-if-v28-4-0a9659c7e69d@nxp.com>
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.
> 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?
> - 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: sashiko-bot @ 2026-07-17 11:12 UTC (permalink / raw)
To: pankaj.gupta; +Cc: conor+dt, imx, devicetree, robh, Frank.Li
In-Reply-To: <20260717-imx-se-if-v28-5-0a9659c7e69d@nxp.com>
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.
[ ... ]
> @@ -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.
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.
[ ... ]
> 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).
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.
[ ... ]
> @@ -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.
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] dt-bindings: i3c: dw: Document missing optional core reset
From: sashiko-bot @ 2026-07-17 11:10 UTC (permalink / raw)
To: Shubham Patil
Cc: linux-i3c, robh, devicetree, Frank.Li, conor+dt,
Alexandre Belloni
In-Reply-To: <20260717105829.567522-1-shubhamsanjay.patil@amd.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The dt-binding specifies the APB clock name as 'apb', but the driver attempts to fetch it using the name 'pclk'.
- [High] Mixing devm_clk_get_enabled() with manual runtime PM clock toggling causes unbalanced clock disables.
--
--- Patch [1]: [PATCH] dt-bindings: i3c: dw: Document missing optional core reset ---
commit 3d9cd37483dab62bf65ce3e70bf1ff17d3bd87d7
Author: Shubham Patil <shubhamsanjay.patil@amd.com>
dt-bindings: i3c: dw: Document missing optional core reset
The DesignWare I3C master IP supports a core reset input, but the
binding is missing the corresponding reset description. Document the
reset property and name in the binding. Keep the reset optional because
the reset line is integration-specific.
> diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> index e803457d3f554..822faf8d221b8 100644
> --- a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> +++ b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> @@ -38,6 +38,12 @@ properties:
> interrupts:
> maxItems: 1
>
[Severity: High]
This is a pre-existing issue, but I noticed a potential mismatch between the
DT binding and the driver for the APB clock while reviewing this file.
The binding enforces the name 'apb':
Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml:
- const: apb
However, the driver requests it by the name 'pclk':
drivers/i3c/master/dw-i3c-master.c:dw_i3c_common_probe() {
...
master->pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk");
...
}
If a device tree is created strictly following the binding using clock-names =
"core", "apb", won't dw_i3c_common_probe() fail to find the "pclk" clock?
Since it is requested as optional, the probe will succeed but subsequent
register accesses will occur without the APB clock enabled. Could this lead to
a synchronous external abort or system freeze during operation?
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + const: core_rst
> +
> power-domains:
> maxItems: 1
[Severity: High]
This is also a pre-existing issue, but I noticed a potential runtime PM
imbalance with how the clocks are handled in the driver.
The clocks are acquired using devres, which ties enablement strictly to the
device lifecycle:
drivers/i3c/master/dw-i3c-master.c:dw_i3c_common_probe() {
...
master->core_clk = devm_clk_get_enabled(&pdev->dev, NULL);
...
master->pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk");
...
}
However, they are manually disabled during runtime suspend:
drivers/i3c/master/dw-i3c-master.c:dw_i3c_master_runtime_suspend() {
...
dw_i3c_master_disable_clks(master);
...
}
And runtime PM is disabled on removal without waking the device:
drivers/i3c/master/dw-i3c-master.c:dw_i3c_common_remove() {
...
pm_runtime_disable(master->dev);
...
}
If the driver is unbound via sysfs while the device is idle in runtime suspend,
the clocks remain disabled. When the devres actions fire during cleanup, won't
they attempt to disable the clocks a second time?
Could this unbalanced clock disable turn off a shared clock and affect other
active peripherals?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717105829.567522-1-shubhamsanjay.patil@amd.com?part=1
^ permalink raw reply
* Re: [PATCH v28 3/7] firmware: imx: add driver for NXP EdgeLock Enclave
From: sashiko-bot @ 2026-07-17 11:10 UTC (permalink / raw)
To: pankaj.gupta; +Cc: Frank.Li, imx, devicetree, conor+dt, robh
In-Reply-To: <20260717-imx-se-if-v28-3-0a9659c7e69d@nxp.com>
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.
> 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.
> + 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.
> 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.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-imx-se-if-v28-0-0a9659c7e69d@nxp.com?part=3
^ permalink raw reply
* Re: [PATCH v28 6/7] arm64: dts: imx8ulp: add secure enclave node
From: sashiko-bot @ 2026-07-17 11:09 UTC (permalink / raw)
To: pankaj.gupta; +Cc: imx, robh, conor+dt, devicetree, Frank.Li
In-Reply-To: <20260717-imx-se-if-v28-6-0a9659c7e69d@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Bypass of the fw_busy circuit breaker when a signal interrupts ele_msg_rcv() at or after the timeout deadline, leading to DMA memory corruption.
- [Medium] The `hsm0` (`secure-enclave`) device tree node is missing the mandatory `memory-region` property, causing device tree schema validation to fail and the driver probe to unconditionally abort.
--
--- Patch [6]: [PATCH v28 6/7] arm64: dts: imx8ulp: add secure enclave node ---
commit 241fbc418ec78fd2d90f05f09b33f7f8fd2d525e
Author: Pankaj Gupta <pankaj.gupta@nxp.com>
arm64: dts: imx8ulp: add secure enclave node
Add support for NXP secure enclave called EdgeLock Enclave firmware (se-fw)
for imx8ulp-evk.
Add label sram0 for sram@2201f000 and add secure-enclave node
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index c6d1bb9edf388..182f1444b6434 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
[ ... ]
> @@ -185,6 +185,13 @@ scmi_sensor: protocol@15 {
> #thermal-sensor-cells = <1>;
> };
> };
> +
> + hsm0: secure-enclave {
> + compatible = "fsl,imx8ulp-se-ele-hsm";
> + mbox-names = "tx", "rx";
> + mboxes = <&s4muap 0 0>, <&s4muap 1 0>;
> + sram = <&sram0>;
> + };
[Severity: Medium]
Does this node need a memory-region property to probe successfully?
Looking at se_if_probe() in drivers/firmware/imx/se_ctrl.c, when the
compatible string has reserved_dma_ranges set to true (as this one does),
it calls of_reserved_mem_device_init().
If the memory-region property is missing, that function returns -ENODEV,
which the driver converts into a probe error:
if (if_node->reserved_dma_ranges) {
ret = of_reserved_mem_device_init(dev);
if (ret)
return dev_err_probe(dev, ret,
"Failed to init reserved memory region.");
}
Will this cause the secure enclave device to unconditionally fail
initialization out-of-the-box?
[Severity: Critical]
Is it possible for the driver's firmware busy circuit breaker to be bypassed
when communicating with this device?
Looking at ele_msg_rcv() in drivers/firmware/imx/ele_common.c, if a signal
interrupts wait_for_completion_interruptible_timeout() at or after the
timeout deadline, an early break bypasses the circuit breaker:
if (is_rsp_wait_with_timeout) {
if (time_after_eq(jiffies, deadline_jiffies)) {
ret = -ETIMEDOUT;
break;
}
remaining_jiffies = deadline_jiffies - jiffies;
}
Because this breaks early, the loop skips the call to se_mark_fw_busy().
Later, in se_dev_ctx_shared_mem_cleanup() in drivers/firmware/imx/se_ctrl.c,
since the fw_busy flag wasn't set, the DMA memory allocator resets the buffer
position to 0:
is_fw_busy_dev_ctx = se_is_fw_busy_ctx(dev_ctx);
/* ... */
if (!is_fw_busy_dev_ctx)
se_shared_mem_mgmt->non_secure_mem.pos = 0;
Could this allow the active firmware DMA to overwrite the memory of
subsequent requests since the firmware is still running and will DMA
to the shared buffer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-imx-se-if-v28-0-0a9659c7e69d@nxp.com?part=6
^ permalink raw reply
* Re: [PATCH v2] of/address: Fix NULL bus dereference in of_pci_range_parser_one()
From: Carlo Caione @ 2026-07-17 11:08 UTC (permalink / raw)
To: Rob Herring, Carlo Caione; +Cc: saravanak, devicetree, linux-kernel, stable
In-Reply-To: <20260716204914.GA1262499-robh@kernel.org>
On Thu Jul 16, 2026 at 10:49 PM CEST, Rob Herring wrote:
> On Mon, Jul 06, 2026 at 01:47:17PM +0200, Carlo Caione wrote:
...
>> ranges = of_get_property(np, "dma-ranges", &len);
>> - if (ranges && len) {
>> - of_dma_range_parser_init(&parser, np);
>> + if (ranges && len && !of_dma_range_parser_init(&parser, np) &&
>> + of_range_count(&parser)) {
>
> Why do we need of_range_count() here?
too zelous but this hopefully covers the case where init succeeds but
the property holds no complete entry, so the loop never runs. E.g. a
32-bit style triplet on a 2/2-cells bus:
soc {
#address-cells = <2>;
#size-cells = <2>;
dma-ranges = <0x0 0x40000000 0x40000000>;
};
It is unlikely and a pre-existing problem so I can drop it if you want.
cheers,
--
Carlo Caione
^ permalink raw reply
* Re: [PATCH v5 02/16] device property: Add fwnode_graph_get_next_port_endpoint()
From: Chen-Yu Tsai @ 2026-07-17 11:05 UTC (permalink / raw)
To: Andy Shevchenko
Cc: sashiko-reviews, devicetree, driver-core, Bartosz Golaszewski
In-Reply-To: <allHTc-bJrPwDA0b@ashevche-desk.local>
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.
> > > [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.
> > > > + 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.
ChenYu
> > 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;
> > > > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* RE: [PATCH v27 5/7] firmware: drivers: imx: adds miscdev
From: Pankaj Gupta (OSS) @ 2026-07-17 11:01 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev, Pankaj Gupta (OSS)
Cc: conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org,
Frank.Li@kernel.org, imx@lists.linux.dev
In-Reply-To: <20260715173648.D6C7C1F000E9@smtp.kernel.org>
Disposition for Sashiko comments on patch 5/7:
> - [High] The `read()` path drops `fops_lock` and `clbk_rx_lock` before
> accessing shared lists and `rx_msg`, leading to Use-After-Free and list
> corruption.
[Accepted]
The read path now reacquires dev_ctx->fops_lock after ele_msg_rcv() returns
and before touching pending shared-memory lists or command-receiver rx_msg
state. The code also rechecks cleanup_done under fops_lock before accessing
the dev_ctx state.
This prevents a concurrent close/unbind path from freeing the DMA backing
memory or pending buffer lists while read() resumes after the blocking wait.
Fixed in v28.
> - [High] KASAN slab-out-of-bounds read in `se_val_rsp_hdr_n_status` due to
> insufficient response buffer size validation.
[Accepted]
se_val_rsp_hdr_n_status() now guards access to msg->data[0]. A response with
only the message header is no longer treated as safe for unconditional status
word access.
The response status word is read only when the response header size indicates
that the message contains data beyond the header.
Fixed in v28.
> - [High] Indefinite hang during module unload or device unbind when userspace
> is blocked in `SE_IOCTL_CMD_SEND_RCV_RSP`.
[Accepted]
The teardown path now wakes the synchronous response waiter before draining
device contexts. This prevents unbind/remove from waiting indefinitely behind a
userspace thread blocked in ele_msg_rcv().
The cleanup path can therefore proceed even when userspace is blocked waiting
for a firmware response.
Fixed in v28.
> - [High] Hung task panic triggered by signal interruption during long firmware
> waits in `ele_msg_rcv()`.
[Accepted]
The default command-response timeout was reduced from 5000 seconds to 3000 ms.
This keeps the signal-interrupted uninterruptible wait below the default
hung-task watchdog threshold while still allowing the protocol to synchronize
after a signal.
Fixed in v28.
> - [High] The `SE_IOCTL_CMD_SEND_RCV_RSP` ioctl silently swallows kernel-level
> validation errors, returning success to userspace.
[Not a concern]
The ioctl handler donot need to propagates se_val_rsp_hdr_n_status() failures back to
userspace. Userspace test-plan includes running negative tests. Userspace itself do those checks.
No need.
> - [High] Memory leak of dynamically allocated `se_buf_desc` structures when a
> device fd is closed without cleaning up pending buffers.
[Accepted]
cleanup_se_shared_mem() now calls se_dev_ctx_shared_mem_cleanup() before
freeing the coherent DMA backing memory. This releases any pending se_buf_desc
entries from pending_in and pending_out when a file descriptor is closed with
pending I/O buffers.
Fixed in v28.
> - [High] IOCTL error path zeroes out shared DMA memory buffers while the
> firmware might still be actively accessing them due to a timeout.
[Accepted]
se_dev_ctx_shared_mem_cleanup() now checks whether the dev_ctx is the active
fw_busy context. If the context is still associated with a timed-out firmware
transaction, the cleanup path avoids zeroing shared DMA buffers because
firmware may still be reading or writing those buffers.
Fixed in v28.
> - [High] Fatal kernel crash during probe failure due to `dma_free_coherent` on
> an uninitialized pointer for the internal device context.
[Accepted]
cleanup_se_shared_mem() now checks whether the coherent memory pointer was
actually allocated before calling dma_free_coherent(). This avoids calling
dma_free_coherent() with a NULL pointer during probe-failure cleanup.
Fixed in v28.
> - [High] Unsynchronized clearing of `rx_msg_sz` in `se_if_fops_read()` causes
> lost commands and `-ENODEV` when interrupted by a signal.
[Accepted]
The command-receiver rx_msg_sz update is now done under
cmd_receiver_clbk_hdl.clbk_rx_lock. The read path also validates
cmd_receiver_clbk_hdl.dev_ctx, rx_msg, and rx_msg_sz under the same callback
lock before consuming the message.
Fixed in v28.
> - [High] TOCTOU race during device context creation in `open()` against unbind
> cleanup allows file operations on an unbound driver.
[Not a concern]
The misc-device open path is protected by the open-gate object and reference
counting. The open gate serializes access to gate->dying and gate->priv. Once
teardown starts, new opens are rejected. If an open already passed the gate, it
holds a valid reference to priv while creating the per-file dev_ctx.
The private misc device context also carries cleanup_done state, which is
checked before creating a new userspace dev_ctx.
No additional change is required.
> - [High] Race window during driver unbind between `cancel_work_sync()` and
> `se_clear_fw_busy()` allows work scheduling on freed memory.
[Accepted]
The teardown order was changed so se_clear_fw_busy() is called before
cancel_work_sync(). This clears the fw_busy state before pending work is
cancelled, preventing a late mailbox interrupt from scheduling fw_busy_work
after teardown has started.
Fixed in v28.
> - [Medium] Race condition exposing device node before probe initialization
> completes.
[Not a concern]
The device intentionally allows baseline firmware communication before runtime
firmware loading. The baseline ELE firmware services, such as SoC information
and firmware management flows, are required during bring-up and are valid
before runtime firmware authentication is complete.
Therefore, exposing the misc device before runtime firmware loading completes
does not bypass required initialization; baseline firmware message exchange is
part of the intended design.
No change is required.
> - [Medium] Mixing `goto`-based error handling with scope-based cleanup
> (`__free` and `guard`) violates kernel cleanup rules.
[Accepted]
The cleanup_dev_ctx() path was refactored to avoid jumping out of a
scoped_guard() block. The cleanup path now uses a flat control flow with a
local state variable instead of a goto from inside scoped cleanup.
Fixed in v28.
^ permalink raw reply
* RE: [PATCH v27 3/7] firmware: imx: add driver for NXP EdgeLock Enclave
From: Pankaj Gupta (OSS) @ 2026-07-17 11:01 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev, Pankaj Gupta (OSS)
Cc: Frank.Li@kernel.org, devicetree@vger.kernel.org, robh@kernel.org,
conor+dt@kernel.org, imx@lists.linux.dev
In-Reply-To: <20260715173654.DFC1F1F00A3E@smtp.kernel.org>
Disposition for Sashiko comments on patch 3/7:
> - [Critical] The `tx_msg` buffer can be freed while a dangling pointer remains in
> the mailbox core's transmit queue, leading to a Use-After-Free read.
[Not a concern]
The i.MX MU mailbox controller copies the message payload into MU registers
synchronously in imx_mu_send_data(). The controller does not retain the tx_msg
pointer after mbox_send_message() returns. Therefore, the caller-owned tx_msg
buffer can be released after a successful mbox_send_message().
An explicit comment will be added in ele_msg_send() to document this lifetime
assumption.
> - [High] A mathematically valid message checksum of `0` is treated as a fatal
> error, causing spontaneous failures in `ele_service_swap()`.
[Accepted]
Updated the checksum helper to return an error code and program the computed
checksum into the message checksum word. A checksum value of 0 is no longer
treated as a helper failure; invalid input is reported through -EINVAL.
This will be fixed in v28.
> - [High] Global static variable `var_se_info.soc_rev` bypasses SoC device
> registration and per-device IMEM initialization on re-probes or secondary MUs.
[Not a concern]
var_se_info.soc_rev stores SoC-level fuse information. The SoC revision is
common for all secure-enclave MU instances on the same SoC and does not vary
per MU.
get_se_soc_info() is only used for secure-enclave firmware instances of type
SE_TYPE_ID_HSM. For platforms with multiple HSM/ELE MUs, the SoC revision,
SoC attributes, and initial IMEM state are common properties of the secure
enclave firmware. Therefore, this information only needs to be fetched once.
The SoC device attributes are registered once, only when se_info->soc_name is
available.
> - [High] Overriding successful firmware transaction results with `-ERESTARTSYS`
> causes non-idempotent commands to be executed twice.
[Not accepted - intentional protocol synchronization behavior]
The signal is not acted on immediately while a firmware message exchange is in
progress. The driver defers signal handling until the firmware transaction is
brought back to a synchronized state. This prevents Linux and firmware from
diverging on message ownership and protocol state.
If a signal arrives while waiting for a response, the wait is continued until
the response path completes. After the protocol state is synchronized, the
syscall reports the interrupted wait. This is intentional and does not cause
the kernel driver to send the command again.
The userspace command/response ABI must treat this as an interrupted operation
after synchronization, not as permission for the kernel driver to re-execute the
command.
> - [High] DMA buffers are freed locally on timeout while the hardware firmware
> transaction is still active, causing background memory corruption.
[Deferred to patch 5/7]
Patch 3/7 still uses local DMA buffers for some internal transactions. Patch
5/7 introduces persistent per-dev_ctx shared memory, including memory for
priv_dev_ctx. The affected internal command paths will be moved from local DMA
buffers to the shared memory owned by priv_dev_ctx.
The timeout/lifetime handling is therefore addressed in patch 5/7, where the
driver can retain the relevant dev_ctx and its DMA memory while firmware may
still be processing a timed-out transaction.
> - [High] The IMEM size returned by the firmware during export is not validated
> against the allocated buffer size, causing an out-of-bounds DMA read during
> resume.
[Accepted]
The exported IMEM size returned by firmware will be validated against
ELE_IMEM_SIZE before it is stored in imem->size and reused during resume-time
IMEM import.
This will be fixed in v28.
> - [High] The driver fails to set a 32-bit coherent DMA mask, despite the hardware
> requiring 32-bit physical addresses for key firmware commands.
[Accepted]
The ELE-accessible memory region is provided through the mandatory
memory-region property on platforms that require constrained DDR access. This
reserved memory region is within the ELE-addressable 32-bit DDR window, and the
message paths also retain upper_32_bits() checks before passing addresses to
firmware.
Even though allocations are expected to come from the reserved 32-bit
ELE-accessible memory region, a 32-bit DMA/coherent mask will also be set to
make the hardware addressing constraint explicit.
This will be fixed in v28.
> - [Medium] Mixing `goto`-based error handling with scope-based `__free()` and
> `guard()` declarations within the same function.
[Accepted]
The affected code will be refactored to avoid mixing goto-based cleanup with
scope-based cleanup in the same function. A static cleanup helper will be added
where appropriate, and the goto cleanup path will be removed.
This will be fixed in v28.
> - [Medium] `ele_msg_rcv()` hardcodes a check against the wrong callback
> handle for deferred signals, breaking abstraction.
[Accepted]
The deferred signal handling check will be updated to use the callback handle
being waited on instead of hardcoding waiting_rsp_clbk_hdl.
The updated condition will check both:
1. the callback handle corresponds to the synchronous response waiter, and
2. READ_ONCE(se_clbk_hdl->rx_msg) is still valid.
This keeps the command-receiver path separate from synchronous response-wait
handling.
This will be fixed in v28.
> - [Medium] Missing NULL check for dynamically allocated attributes in
> `get_se_soc_info()`.
[Accepted]
The return values from devm_kasprintf() will be checked before passing the
allocated strings to soc_device_register().
This will be fixed in v28.
> - [Medium] Data race on global variable `var_se_info` during concurrent device
> probes.
[Accepted]
A mutex will be added around population of var_se_info.soc_rev. Once the lock is
acquired, the driver will check whether soc_rev has already been populated. If
it has been populated, the function can skip re-reading the SoC fuse data.
var_se_info.soc_rev stores immutable SoC-level fuse information and is only
populated once. After that, it is only referenced.
This will be fixed in v28.
> - [Low] Residual error check for `se_fill_cmd_msg_hdr` remains in
> `ele_service_swap` despite the changelog explicitly claiming its removal.
[Accepted]
The stale error check will be removed. se_fill_cmd_msg_hdr() now returns void,
so the remaining ret check is leftover dead code from the earlier refactor.
This will be fixed in v28.
^ permalink raw reply
* Re: [PATCH v28 1/7] Documentation/firmware: add imx/se to other_interfaces
From: sashiko-bot @ 2026-07-17 11:00 UTC (permalink / raw)
To: pankaj.gupta; +Cc: conor+dt, imx, robh, Frank.Li, devicetree
In-Reply-To: <20260717-imx-se-if-v28-1-0a9659c7e69d@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The documentation references a missing file `drivers/firmware/imx/se_ctrl.c`.
- [Low] Multiple grammatical and phrasing issues in the newly added documentation hinder readability.
--
--- Patch [1]: [PATCH v28 1/7] Documentation/firmware: add imx/se to other_interfaces ---
commit d14736a873782e28bc9a614142ccc96cf497924b
Author: Pankaj Gupta <pankaj.gupta@nxp.com>
Documentation/firmware: add imx/se to other_interfaces
This commit documents the i.MX SoC Service layer and character device
driver for SoCs containing NXP hardware IPs for Secure Enclaves. It
details features like the NXP EdgeLock Enclave on i.MX93 and i.MX8ULP.
> diff --git a/Documentation/driver-api/firmware/other_interfaces.rst b/Documentation/driver-api/firmware/other_interfaces.rst
> index 06ac89adaafba..6c6fa9a0ba1df 100644
> --- a/Documentation/driver-api/firmware/other_interfaces.rst
> +++ b/Documentation/driver-api/firmware/other_interfaces.rst
> @@ -49,3 +49,136 @@ of the requests on to a secure monitor (EL3).
>
> .. kernel-doc:: drivers/firmware/stratix10-svc.c
> :export:
> +
> +NXP Secure Enclave Firmware Interface
> +=====================================
> +
> +Introduction
> +------------
> +The NXP's i.MX HW IP like EdgeLock Enclave, V2X etc., creates an embedded secure
> +enclave within the SoC boundary to enable features like:
[Severity: Low]
This isn't a bug, but does this documentation contain several awkward
phrases and grammatical errors? Phrases like "The NXP's i.MX HW IP like",
"enabled with the such secure enclaves", and "there is/are dedicated" might
make this difficult to read. Could this section be reworded for clarity?
[ ... ]
> +When a user sends a command to the firmware, it registers its device_ctx
> +as waiter of a response from firmware.
> +
> +Enclave's Firmware owns the storage management over a Linux filesystem.
> +For this c_dev provisions a dedicated slave device called "receiver".
> +
> +.. kernel-doc:: drivers/firmware/imx/se_ctrl.c
> + :export:
[Severity: Low]
Does this file actually exist in the repository at this commit? Generating
the kernel documentation will likely result in a build warning or error
because the source file drivers/firmware/imx/se_ctrl.c cannot be found.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-imx-se-if-v28-0-0a9659c7e69d@nxp.com?part=1
^ permalink raw reply
* [PATCH v2 5/5] arm64: dts: qcom: talos-evk-som: Fix BT RFA supply name
From: Konrad Dybcio @ 2026-07-17 11:00 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, André Apitzsch, Luca Weiss, Gabriela David
Cc: linux-arm-msm, devicetree, linux-kernel, Konrad Dybcio,
Krzysztof Kozlowski
In-Reply-To: <20260717-topic-june26_dts_fixes-v2-0-797cd46e5d9f@oss.qualcomm.com>
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Fix up the supply name to align with qcom,wcn6855-bt bindings.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/talos-evk-som.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/talos-evk-som.dtsi b/arch/arm64/boot/dts/qcom/talos-evk-som.dtsi
index 6d2cff3cf23c..a057a7271480 100644
--- a/arch/arm64/boot/dts/qcom/talos-evk-som.dtsi
+++ b/arch/arm64/boot/dts/qcom/talos-evk-som.dtsi
@@ -535,7 +535,7 @@ bluetooth {
vddbtcmx-supply = <&vreg_pmu_btcmx_0p85>;
vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
- vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>;
+ vddrfa1p7-supply = <&vreg_pmu_rfa_1p7>;
};
};
--
2.55.0
^ permalink raw reply related
* [PATCH v2 4/5] arm64: dts: qcom: qcs8300-ride: Fix BT RFA supply name
From: Konrad Dybcio @ 2026-07-17 11:00 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, André Apitzsch, Luca Weiss, Gabriela David
Cc: linux-arm-msm, devicetree, linux-kernel, Konrad Dybcio,
Krzysztof Kozlowski
In-Reply-To: <20260717-topic-june26_dts_fixes-v2-0-797cd46e5d9f@oss.qualcomm.com>
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Fix up the supply name to align with qcom,wcn6855-bt bindings.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
index e9a8553a8d82..1ff39530ea3d 100644
--- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
@@ -837,7 +837,7 @@ bluetooth: bluetooth {
vddbtcmx-supply = <&vreg_pmu_btcmx_0p85>;
vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
- vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>;
+ vddrfa1p7-supply = <&vreg_pmu_rfa_1p7>;
};
};
--
2.55.0
^ permalink raw reply related
* [PATCH v2 3/5] arm64: dts: qcom: sdm632-motorola-ocean: Fix LED default trigger property
From: Konrad Dybcio @ 2026-07-17 11:00 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, André Apitzsch, Luca Weiss, Gabriela David
Cc: linux-arm-msm, devicetree, linux-kernel, Konrad Dybcio,
Krzysztof Kozlowski
In-Reply-To: <20260717-topic-june26_dts_fixes-v2-0-797cd46e5d9f@oss.qualcomm.com>
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The correct property name is "linux,default-trigger", not
"default-trigger". Fix it to avoid DT checker warnings and let the OSes
consume the intended information.
Fixes: 3176c4d6b9be ("arm64: dts: qcom: sdm632: Add device tree for Motorola G7 Power")
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sdm632-motorola-ocean.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm632-motorola-ocean.dts b/arch/arm64/boot/dts/qcom/sdm632-motorola-ocean.dts
index 2f55db0c8ce3..9ea3e5e76bf9 100644
--- a/arch/arm64/boot/dts/qcom/sdm632-motorola-ocean.dts
+++ b/arch/arm64/boot/dts/qcom/sdm632-motorola-ocean.dts
@@ -130,7 +130,7 @@ led-controller@36 {
led: led@1 {
reg = <1>;
- default-trigger = "backlight";
+ linux,default-trigger = "backlight";
function = LED_FUNCTION_BACKLIGHT;
led-sources = <0 1 2>;
};
--
2.55.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox