* Re: [PATCH v4 06/13] clk: qcom: dispcc-qcm2290: Switch to DT index based clk lookup
From: Imran Shaik @ 2026-06-21 13:30 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio, Loic Poulain,
Brian Masney, Ajit Pandey, Taniya Das, Jagadeesh Kona,
linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <20260605-aspiring-ambrosial-squirrel-a7ef63@quoll>
On 05-06-2026 03:58 pm, Krzysztof Kozlowski wrote:
> On Thu, Jun 04, 2026 at 10:56:12AM +0530, Imran Shaik wrote:
>> Update the QCM2290 DISPCC driver to use the DT index based parent clock
>> lookup to align with the latest convention. While at it, fix the parent
>> data of mdss ahb/mdp clocks to use GPLL0 main output as per HW clock plan,
>> and update frequency table accordingly. Also, add the DSI1 PHY PLL input
>> clocks support.
>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>> drivers/clk/qcom/dispcc-qcm2290.c | 44 ++++++++++++++++++++++++++-------------
>> 1 file changed, 30 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
>> index 1c21267ae0f7a86c1de88e888c2a990c35f0a0e0..f5dbd19d0a0334362a44f91a69229cb0f018f309 100644
>> --- a/drivers/clk/qcom/dispcc-qcm2290.c
>> +++ b/drivers/clk/qcom/dispcc-qcm2290.c
>> @@ -24,6 +24,18 @@
>> #include "gdsc.h"
>> #include "reset.h"
>>
>> +enum {
>> + DT_BI_TCXO,
>> + DT_BI_TCXO_AO,
>> + DT_GPLL0_OUT_DIV,
>> + DT_GPLL0,
>> + DT_DSI0_PHY_PLL_OUT_BYTECLK,
>> + DT_DSI0_PHY_PLL_OUT_DSICLK,
>> + DT_DSI1_PHY_PLL_OUT_BYTECLK,
>> + DT_DSI1_PHY_PLL_OUT_DSICLK,
>> + DT_SLEEP_CLK,
>> +};
>> +
>> enum {
>> P_BI_TCXO,
>> P_BI_TCXO_AO,
>> @@ -33,6 +45,8 @@ enum {
>> P_GPLL0_OUT_DIV,
>> P_GPLL0_OUT_MAIN,
>> P_SLEEP_CLK,
>> + P_DSI1_PHY_PLL_OUT_BYTECLK,
>> + P_DSI1_PHY_PLL_OUT_DSICLK,
>
> You just added new parents which looks like clear ABI break disguised as
> "switch to DT".
>
> First, don't mix independent changes. Second, ABI changes must be
> clearly documented and your first commits even tell false statements
> claiming there is juet "extension".
>
> Did you test THIS EXACTLY patch on existing devices?
>
Yes functionally it works, as these missing external DSI1 clocks are not
used but present in the DISPCC hardware. I will split the patches as
suggested.
Thanks,
Imran
^ permalink raw reply
* Re: [PATCH v4 07/13] clk: qcom: dispcc-qcm2290: Update GDSC *wait_val values and flags
From: Imran Shaik @ 2026-06-21 13:30 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio, Loic Poulain,
Brian Masney, Ajit Pandey, Taniya Das, Jagadeesh Kona,
linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <66iobdkznpqqy7twf4fhu33m7mclnndlrjc4urt2em6tfe6gp4@r7wd2gnzsjoa>
On 06-06-2026 04:57 pm, Dmitry Baryshkov wrote:
> On Thu, Jun 04, 2026 at 10:56:13AM +0530, Imran Shaik wrote:
>> Update the QCM2290 DISPCC GDSC wait_val fields to match the hardware
>> default values. Incorrect settings can cause the GDSC FSM to stuck,
>> leading to power on/off failures. And update GDSC flags to retain the
>> registers, and poll for the CFG GDSCR, and switch between HW/SW mode
>> dynamically as per the latest convention.
>
> Too many ands for one patch. Zero explanation (other than 'latest
> convention'. Which convention? The flags describe hardware behaviour,
> not conventions).
>
Sure, I will split the patches and update the commit text with the
justification details.
Thanks,
Imran
>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>> drivers/clk/qcom/dispcc-qcm2290.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
>> index f5dbd19d0a0334362a44f91a69229cb0f018f309..4c1eef79f41b6907fe79f2b18bcb5f6160c74a43 100644
>> --- a/drivers/clk/qcom/dispcc-qcm2290.c
>> +++ b/drivers/clk/qcom/dispcc-qcm2290.c
>> @@ -468,11 +468,14 @@ static const struct qcom_reset_map disp_cc_qcm2290_resets[] = {
>>
>> static struct gdsc mdss_gdsc = {
>> .gdscr = 0x3000,
>> + .en_rest_wait_val = 0x2,
>> + .en_few_wait_val = 0x2,
>> + .clk_dis_wait_val = 0xf,
>> .pd = {
>> .name = "mdss_gdsc",
>> },
>> .pwrsts = PWRSTS_OFF_ON,
>> - .flags = HW_CTRL,
>> + .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
>> };
>>
>> static struct gdsc *disp_cc_qcm2290_gdscs[] = {
>>
>> --
>> 2.34.1
>>
>
^ permalink raw reply
* Re: [PATCH v4 08/13] clk: qcom: gpucc-qcm2290: Move to the latest common qcom_cc_probe() model
From: Imran Shaik @ 2026-06-21 13:31 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio, Loic Poulain,
Brian Masney, Ajit Pandey, Taniya Das, Jagadeesh Kona,
linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <4zhwpr2n6ihjevwamqzutoyoj4owg3mkutbn3ddstooh566fqy@j2k2or3narkd>
On 06-06-2026 04:59 pm, Dmitry Baryshkov wrote:
> On Thu, Jun 04, 2026 at 10:56:14AM +0530, Imran Shaik wrote:
>> Update the QCM2290 GPUCC driver to use the qcom_cc_probe() model by moving
>> the critical clocks handling and PLL configurations from probe to the
>> driver_data to align with the latest convention. While at it, drop the
>> modelling of gpu_cc_ahb_clk and gpu_cc_cxo_aon_clk clocks and keep them
>
> Why? Also it looks like a separate commit to me.
>
Sure, I will check and split the patches.
>> enabled from probe as per the hardware requirements, and drop pm_clk
>> handling as the required GCC clocks are kept always-on from GCC probe.
>
> Separate commit.
>
Sure.
>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>> drivers/clk/qcom/gpucc-qcm2290.c | 92 +++++++++-------------------------------
>> 1 file changed, 21 insertions(+), 71 deletions(-)
>>
>> @@ -224,19 +210,6 @@ static struct clk_branch gpu_cc_cx_snoc_dvm_clk = {
>> },
>> };
>>
>> -static struct clk_branch gpu_cc_cxo_aon_clk = {
>> - .halt_reg = 0x1004,
>> - .halt_check = BRANCH_HALT_DELAY,
>> - .clkr = {
>> - .enable_reg = 0x1004,
>> - .enable_mask = BIT(0),
>> - .hw.init = &(struct clk_init_data){
>> - .name = "gpu_cc_cxo_aon_clk",
>> - .ops = &clk_branch2_ops,
>> - },
>> - },
>> -};
>
> This wasn't always-on beforehand. Why is being changed?
>
>> -
>> static struct clk_branch gpu_cc_cxo_clk = {
>> .halt_reg = 0x109c,
>> .halt_check = BRANCH_HALT,
>> @@ -318,12 +291,10 @@ static struct gdsc gpu_gx_gdsc = {
>> };
>>
>> static struct clk_regmap *gpu_cc_qcm2290_clocks[] = {
>> - [GPU_CC_AHB_CLK] = &gpu_cc_ahb_clk.clkr,
>> [GPU_CC_CRC_AHB_CLK] = &gpu_cc_crc_ahb_clk.clkr,
>> [GPU_CC_CX_GFX3D_CLK] = &gpu_cc_cx_gfx3d_clk.clkr,
>> [GPU_CC_CX_GMU_CLK] = &gpu_cc_cx_gmu_clk.clkr,
>> [GPU_CC_CX_SNOC_DVM_CLK] = &gpu_cc_cx_snoc_dvm_clk.clkr,
>> - [GPU_CC_CXO_AON_CLK] = &gpu_cc_cxo_aon_clk.clkr,
>> [GPU_CC_CXO_CLK] = &gpu_cc_cxo_clk.clkr,
>> [GPU_CC_GMU_CLK_SRC] = &gpu_cc_gmu_clk_src.clkr,
>> [GPU_CC_GX_GFX3D_CLK] = &gpu_cc_gx_gfx3d_clk.clkr,
>> @@ -342,6 +313,16 @@ static struct gdsc *gpu_cc_qcm2290_gdscs[] = {
>> [GPU_GX_GDSC] = &gpu_gx_gdsc,
>> };
>>
>> +static struct clk_alpha_pll *gpu_cc_qcm2290_plls[] = {
>> + &gpu_cc_pll0,
>> +};
>> +
>> +static const u32 gpu_cc_qcm2290_critical_cbcrs[] = {
>> + 0x1078, /* GPU_CC_AHB_CLK */
>> + 0x1004, /* GPU_CC_CXO_AON_CLK */
>> + 0x1060, /* GPU_CC_GX_CXO_CLK */
>> +};
>> +
>> static const struct regmap_config gpu_cc_qcm2290_regmap_config = {
>> .reg_bits = 32,
>> .reg_stride = 4,
>> @@ -350,6 +331,12 @@ static const struct regmap_config gpu_cc_qcm2290_regmap_config = {
>> .fast_io = true,
>> };
>>
>> +static const struct qcom_cc_driver_data gpu_cc_qcm2290_driver_data = {
>> + .alpha_plls = gpu_cc_qcm2290_plls,
>> + .num_alpha_plls = ARRAY_SIZE(gpu_cc_qcm2290_plls),
>> + .clk_cbcrs = gpu_cc_qcm2290_critical_cbcrs,
>> + .num_clk_cbcrs = ARRAY_SIZE(gpu_cc_qcm2290_critical_cbcrs),
>> +};
>>
>> static const struct qcom_cc_desc gpu_cc_qcm2290_desc = {
>> .config = &gpu_cc_qcm2290_regmap_config,
>> @@ -359,6 +346,7 @@ static const struct qcom_cc_desc gpu_cc_qcm2290_desc = {
>> .num_resets = ARRAY_SIZE(gpu_cc_qcm2290_resets),
>> .gdscs = gpu_cc_qcm2290_gdscs,
>> .num_gdscs = ARRAY_SIZE(gpu_cc_qcm2290_gdscs),
>> + .driver_data = &gpu_cc_qcm2290_driver_data,
>> };
>>
>> static const struct of_device_id gpu_cc_qcm2290_match_table[] = {
>> @@ -369,45 +357,7 @@ MODULE_DEVICE_TABLE(of, gpu_cc_qcm2290_match_table);
>>
>> static int gpu_cc_qcm2290_probe(struct platform_device *pdev)
>> {
>> - struct regmap *regmap;
>> - int ret;
>> -
>> - regmap = qcom_cc_map(pdev, &gpu_cc_qcm2290_desc);
>> - if (IS_ERR(regmap))
>> - return PTR_ERR(regmap);
>> -
>> - ret = devm_pm_runtime_enable(&pdev->dev);
>
> So, it was pm_runtime-enabled beforehand. Now you've silently dropped
> it.
>
My bad, will add use_rpm flag.
Thanks,
Imran
^ permalink raw reply
* Re: [PATCH v4 09/13] clk: qcom: gpucc-qcm2290: Park RCG's clk source at XO during disable
From: Imran Shaik @ 2026-06-21 13:32 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio,
Loic Poulain, Brian Masney
Cc: Ajit Pandey, Taniya Das, Jagadeesh Kona, linux-arm-msm, linux-clk,
devicetree, linux-kernel
In-Reply-To: <55a34f28-26ab-4bba-8a0d-282b5fadd374@oss.qualcomm.com>
On 10-06-2026 07:30 pm, Konrad Dybcio wrote:
> On 6/4/26 7:26 AM, Imran Shaik wrote:
>> The RCG's clk src has to be parked at XO while disabling as per the
>> HW recommendation, hence use clk_rcg2_shared_ops to achieve the same.
>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>
> This RCG isn't marked as "safe" downstream, was it overlooked there
> too?
>
Apologies for the late reply.
In Shikra downstream it is marked as "safe" properly, but in Agatti
seems it is overlooked.
> In any case, this needs a Fixes tag
>
Sure, will add the Fixes tag in the next series.
Thanks,
Imran
^ permalink raw reply
* Re: [PATCH v4 10/13] clk: qcom: gpucc-qcm2290: Update GDSC *wait_val values and flags
From: Imran Shaik @ 2026-06-21 13:32 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio, Loic Poulain,
Brian Masney, Ajit Pandey, Taniya Das, Jagadeesh Kona,
linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <xcujjo7qdgoupayq5zrddl7gii7s2ljhunvav2jjzynykg7za4@ifykyjburvea>
On 06-06-2026 05:01 pm, Dmitry Baryshkov wrote:
> On Thu, Jun 04, 2026 at 10:56:16AM +0530, Imran Shaik wrote:
>> Update the QCM2290 GPUCC GDSC wait_val fields to match the hardware default
>> values. Incorrect settings can cause the GDSC FSM to stuck, leading to
>> power on/off failures. And update the GPUCC GDSC flags to retain the
>> registers, and poll for the CFG GDSCR as applicable.
>
> Separate commits, explanations, justification, etc. E.g. why are the
> flags being set only to the one of the GDSCs?
>
Sure Dmitry, I will split the patches and update the commit text with
justification details in the next series.
>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>> drivers/clk/qcom/gpucc-qcm2290.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>
^ permalink raw reply
* [PATCH] dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
From: Aditya Chari @ 2026-06-21 8:52 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, gregkh
Cc: jacobsfeder, devicetree, linux-staging, linux-kernel,
Aditya Chari
The axis-fifo driver's compatible strings were undocumented, flagged
by checkpatch.pl as UNDOCUMENTED_DT_STRING. Add a devicetree
binding document describing the required properties
for drivers/staging/axis-fifo, derived from the properties
the driver reads via of_property_read_u32()
in axis_fifo_parse_dt().
Signed-off-by: Aditya Chari <adi25charis@gmail.com>
---
.../bindings/misc/xlnx,axi-fifo-mm-s.yaml | 95 +++++++++++++++++++
1 file changed, 95 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
diff --git a/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
new file mode 100644
index 000000000..8ef0adde3
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/xlnx,axi-fifo-mm-s.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx AXI-Stream FIFO (axis-fifo)
+
+maintainers:
+ - Jacob Feder <jacobsfeder@gmail.com>
+
+description:
+ The AXI-Stream FIFO (AXIS-FIFO) IP core provides a memory-mapped AXI4-Lite
+ interface for sending and receiving data over an AXI4-Stream interface
+ using FIFO buffers. See Xilinx PG080 for IP details.
+
+properties:
+ compatible:
+ enum:
+ - xlnx,axi-fifo-mm-s-4.1
+ - xlnx,axi-fifo-mm-s-4.2
+ - xlnx,axi-fifo-mm-s-4.3
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ xlnx,axi-str-rxd-tdata-width:
+ description:
+ Width in bits of the AXI4-Stream receive data interface. Only a
+ width of 32 is currently supported by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 32
+
+ xlnx,axi-str-txd-tdata-width:
+ description:
+ Width in bits of the AXI4-Stream transmit data interface. Only a
+ width of 32 is currently supported by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 32
+
+ xlnx,rx-fifo-depth:
+ description:
+ Depth in bytes of the receive FIFO, as configured in the IP core.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,tx-fifo-depth:
+ description:
+ Depth in bytes of the transmit FIFO, as configured in the IP core.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,use-rx-data:
+ description:
+ Indicates whether the receive data path is present, as configured
+ in the IP core. A value of 1 enables the receive path, 0 disables it.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+
+ xlnx,use-tx-data:
+ description:
+ Indicates whether the transmit data path is present, as configured
+ in the IP core. A value of 1 enables the transmit path, 0 disables it.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - xlnx,axi-str-rxd-tdata-width
+ - xlnx,axi-str-txd-tdata-width
+ - xlnx,rx-fifo-depth
+ - xlnx,tx-fifo-depth
+ - xlnx,use-rx-data
+ - xlnx,use-tx-data
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ axi_fifo_mm_s_0: axi-fifo-mm-s@40000000 {
+ compatible = "xlnx,axi-fifo-mm-s-4.1";
+ reg = <0x40000000 0x10000>;
+ interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>;
+ xlnx,axi-str-rxd-tdata-width = <32>;
+ xlnx,axi-str-txd-tdata-width = <32>;
+ xlnx,rx-fifo-depth = <2048>;
+ xlnx,tx-fifo-depth = <2048>;
+ xlnx,use-rx-data = <1>;
+ xlnx,use-tx-data = <1>;
+ };
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
From: sashiko-bot @ 2026-06-21 13:43 UTC (permalink / raw)
To: Aditya Chari; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260621085209.52651-1-adi25charis@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] The YAML binding incorrectly documents the FIFO depth properties as being in 'bytes', while the driver and hardware expect them in 'words'.
- [Medium] The schema omits valid hardware properties while enforcing `additionalProperties: false`, falsely restricting the device tree to only the properties the current driver implementation reads and causing valid hardware device trees to fail schema validation.
- [Medium] Hardware data width properties are artificially constrained to `const: 32` based on driver support rather than hardware capabilities.
- [Low] The previous text-based binding document was not removed.
Pre-existing issues:
- [Critical] A pre-existing use-after-free vulnerability occurs if the device is unbound while userspace holds an open file descriptor.
--
commit 1499d3b34c3ed26799e82ed6630ba1f50a8aec04
Author: Aditya Chari <adi25charis@gmail.com>
dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
The axis-fifo driver's compatible strings were undocumented, flagged
by checkpatch.pl as UNDOCUMENTED_DT_STRING. Add a devicetree
binding document describing the required properties
for drivers/staging/axis-fifo, derived from the properties
the driver reads via of_property_read_u32()
in axis_fifo_parse_dt().
> dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
[Severity: Low]
Should the previous text-based binding document be removed in this patch?
It looks like the legacy drivers/staging/axis-fifo/axis-fifo.txt binding
was left in the tree, leaving two conflicting sources of truth for the
same hardware compatible string.
> diff --git a/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
> new file mode 100644
> index 0000000000000..8ef0adde32ad8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
[ ... ]
> + xlnx,axi-str-rxd-tdata-width:
> + description:
> + Width in bits of the AXI4-Stream receive data interface. Only a
> + width of 32 is currently supported by the driver.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + const: 32
> +
> + xlnx,axi-str-txd-tdata-width:
> + description:
> + Width in bits of the AXI4-Stream transmit data interface. Only a
> + width of 32 is currently supported by the driver.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + const: 32
[Severity: Medium]
Should this schema describe the hardware capabilities rather than the software
limitations?
Over-constraining the schema to const: 32 based on current driver support
might cause validation errors for device trees that accurately represent
hardware synthesized with different bus widths.
> + xlnx,rx-fifo-depth:
> + description:
> + Depth in bytes of the receive FIFO, as configured in the IP core.
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + xlnx,tx-fifo-depth:
> + description:
> + Depth in bytes of the transmit FIFO, as configured in the IP core.
> + $ref: /schemas/types.yaml#/definitions/uint32
[Severity: High]
Is it correct to document these properties as being in bytes?
Looking at the driver code, it evaluates the depth as word counts when checking
for overflow:
drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_write() {
...
if (words_to_write > (fifo->tx_fifo_depth - 4)) {
...
}
...
}
If a devicetree author specifies the depth in bytes per the new YAML
documentation, could the driver load a value four times too large, allowing
out-of-bounds writes that might trigger a hardware transmit overrun error?
[ ... ]
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - xlnx,axi-str-rxd-tdata-width
> + - xlnx,axi-str-txd-tdata-width
> + - xlnx,rx-fifo-depth
> + - xlnx,tx-fifo-depth
> + - xlnx,use-rx-data
> + - xlnx,use-tx-data
> +
> +additionalProperties: false
[Severity: Medium]
Does this schema artificially restrict the devicetree by omitting valid
hardware properties?
The original text binding allowed properties such as xlnx,has-axis-tkeep
and interrupt-names. Because the schema enforces additionalProperties: false,
will running dt_binding_check on existing, valid device trees generated for
this IP result in false-positive validation errors?
[Severity: Critical]
This is a pre-existing issue, but does the driver have a use-after-free
vulnerability if the device is unbound while a userspace process holds an
open file descriptor?
In the driver, the axis_fifo structure is allocated with devm_kzalloc() and
I/O memory mapped with devm_platform_get_and_ioremap_resource():
drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_probe() {
...
fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
...
}
However, axis_fifo_remove() unregisters the character device without tracking
or waiting for active file descriptors to close:
drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_remove() {
...
misc_deregister(&fifo->miscdev);
...
}
If the device is unbound via sysfs while a userspace process keeps the char
device open, the driver core's devres cleanup automatically frees the fifo
structure and unmaps base_addr. Would subsequent read(), write(), or poll()
calls via the active file descriptor then access the freed fifo pointer and
unmapped I/O memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260621085209.52651-1-adi25charis@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v4 4/4] iio: flow: add Sensirion SLF3S liquid flow sensor driver
From: Jonathan Cameron @ 2026-06-21 13:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Wadim Mueller, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
David Lechner, Nuno Sá, Andy Shevchenko, Maxwell Doose,
linux-iio, devicetree, linux-kernel, Marcelo Schmitt,
Rodrigo Alencar
In-Reply-To: <84503093-4bff-4c93-aff8-aa07e1a6a1a1@kernel.org>
On Mon, 15 Jun 2026 06:27:00 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 14/06/2026 17:10, Jonathan Cameron wrote:
> > On Sat, 13 Jun 2026 09:51:13 +0200
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> >> On 12/06/2026 19:47, Jonathan Cameron wrote:
> >>> On Thu, 11 Jun 2026 16:01:12 +0200
> >>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>
> >>>> On 11/06/2026 15:27, Wadim Mueller wrote:
> >>>>> +
> >>>>> +static const struct of_device_id slf3s_of_match[] = {
> >>>>> + { .compatible = "sensirion,slf3s-0600f", .data = &slf3s_variants[0] },
> >>>>> + { .compatible = "sensirion,slf3s-1300f", .data = &slf3s_variants[1] },
> >>>>> + { .compatible = "sensirion,slf3s-4000b", .data = &slf3s_variants[2] },
> >>>>
> >>>> You should have only 1300f here and detect the variants. That was my
> >>>> point when I suggested to use the fallback.
> >>>>
> >>>
> >>> I'm lost. How does that work? They cannot fallback to that part because
> >>> it relies on in driver detection of the fact that they are incompatible.
> >>>
> >>
> >> I am lost too. Then why were they made compatible in the binding?
> >>
> >> Entire discussion was that these are FULLY compatible due to variant
> >> detection. That was the entire point of discussing more generic
> >> fallback. Using specific fallback does not change that - it is the same
> >> concept.
> >>
> >> If devices are not detectable, why were we discussing any compatibility?
> >
> > They are detectable, but the feature set is not, so to me there is zero valid
> > in a generic fallback, we have to update the driver every time a new part comes
> > along. (i.e. I agree with Conor's reply to the previous version thread).
> > A specific fallback to a completely compatible part would be fine as there
> > would be sufficient info to not need the ID lookup.
>
> This patch has specific fallback and we discuss this now.
>
> >
> > The case in the binding for this version is the worst of all options because
> > it implies it is valid to fallback to something that gives a false impression
> > of being specific when it's relying on ID matching to say actually it's something
> > else.
> >
> > So definitely not
> > + compatible:
> > + oneOf:
> > + - const: sensirion,slf3s-1300f
> > + - items:
> > + - enum:
> > + - sensirion,slf3s-0600f
> > + - sensirion,slf3s-4000b
> > + - const: sensirion,slf3s-1300f
> > +
> >
> > Falling back to slf3s is better than this, but I'd rather not have a fallback
> > at all, thus allowing correct fallback to the parts listed here in future.
>
> Why? These devices are fully detectable thus fully compatible.
Ok. We aren't really making progress. Let me layout the three approaches.
+ advantages and disadvantages as I understand it.
What we have here:
compatible:
oneOf:
- const: sensirion,slf3s-1300f
- items:
- enum:
- sensirion,slf3s-0600f
- sensirion,slf3s-4000b
- const: sensirion,slf3s-1300f
Advantages: Fallback to a real part.
Disadvantages:
- Requires a driver change even for drop in replacements (which will have
different WHOAMI IDs.
- To me it implies that a driver that supports slf3s-1300f should just
work with parts we haven't seen yet if they have that in their fallback
lists. I appreciate you think that we shouldn't put effort into supporting
that case, but to me it seems more or less free to support and is useful.
Generic compatible for fallback.
compatible:
- items:
- enum:
- sensirion,slf3s-0600f
- sensirion,slf3s-1300f
- sensirion,slf3s-4000b
- const: sensirion,slf3s
Advantages: Fallback to something that doesn't imply the post fix is coming from
DT. So kind of hints we need detection code.
Disadvantages: requires driver changes for each fully compatible drop in part.
Note I'm not sure what this brings us over just having one compatible... Maybe
we'll some day get a non detectable difference where we need to use the binding
but seems fairly unlikely (I've seen that happen once when a manufacturer failed
to update a WHOAMI reg value for a new part).
No fallbacks
compatible:
enum:
- sensirion,slf3s-0600f
- sensirion,slf3s-1300f
- sensirion,slf3s-4000b
Advantages: (Common!) a future drop in compatible part works with old kernels
as the driver knows a match + part information to use if the ID
match fails.
Disadvantages: Not sure I see any... However I'm guessing I'm missing something.
Note for this topic I think it would be great to capture some best practice
information somewhere so we can improve consistency. I've looked around and
there are some bits of feedback in numerous threads but those are hard to track
down.
The Dt spec refers to these as being about programming mode + driver selection.
I will note that we have very broad families of devices that have a somewhat
standard register interface + do always put the WHOAMI in the same place.
In the vendor drivers those match a single hydra of a driver; upstream we
have multiple drivers as devices do very different things and hence in
practice almost no code would have been shared - so I have no idea what
we should have done for those more commplex cases. Following your logic
(I may have it wrong!) I think we would have had a generic compatible
for the whole family but in upstream Linux no driver would have bound
to that. I guess there are different extremes:
1) This case - register interfaces is consistent for all registers,
we are just matching on constant tables needed to interpret the
data in those tables.
2) Similarish. Main register interface shared, but optional additional
parts. If one device is a strict superset, a fallback is
appropriate (in my view) but if I follow your points here then they
should all have a single fallback compatible.
3) Not particularly similar, but still detectable in a consistent
way. To me not appropriate to have a common fallback as based on
the dt-spec thing about matching drivers, they will typically
match different drivers. Maybe we should have a common fallback
but typically not bind to it?
4) Detection code becomes a mess and is different for each part
(see the old i2c_detect stuff) So hopefully no one thinks we
should use a fallback once we reach
this level of complexity (unless part are genuinely compatible)
I'm a little bothered by the fuzzy boundaries between those cases
so would ideally like rules that say 1-3 need fallbacks or none do
but we live in a fuzzy world, so maybe best we can do is leave things
flexible.
Jonathan
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH 2/5] iio: adc: Add ti-ads1262 driver
From: Jonathan Cameron @ 2026-06-21 14:25 UTC (permalink / raw)
To: Kurt Borja
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Linus Walleij,
Bartosz Golaszewski, David Lechner, Nuno Sá, Andy Shevchenko,
linux-iio, devicetree, linux-kernel, linux-gpio
In-Reply-To: <DJ91ZV8FQOMZ.YLIC552K4G5D@gmail.com>
On Sun, 14 Jun 2026 15:27:11 -0500
"Kurt Borja" <kuurtb@gmail.com> wrote:
> Hi Jonathan,
>
> On Sat Jun 13, 2026 at 8:45 AM -05, Jonathan Cameron wrote:
> > On Fri, 12 Jun 2026 17:46:20 -0500
> > Kurt Borja <kuurtb@gmail.com> wrote:
> >
> >> Add ti-ads1262 driver for TI ADS1262 and ADS1263 ADCs with initial
> >> support for the following features:
> >>
> >> - Power management
> >> - IIO direct and buffer modes
> >> - Channel hot-reloading
> >> - Internal or external oscillator
> >> - Internal or external voltage reference
> >> - Filter configuration
> >> - Sensor bias configuration
> >> - IDAC configuration
> >> - Level-shift voltage configuration
> >> - Auxiliary ADC interoperability considerations
> >>
> >> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> >> ---
> >> MAINTAINERS | 1 +
> >> drivers/iio/adc/Kconfig | 13 +
> >> drivers/iio/adc/Makefile | 1 +
> >> drivers/iio/adc/ti-ads1262.c | 1816 ++++++++++++++++++++++++++++++++++++++++++
> >
> > That is rather too big. I think you'll have to work out how to split this
> > up into more manageable chunks. Staying under a 1000 (preferably a lot less)
> > per patch makes it much easier for people to review.
> >
> > Given the complexity of the device this might be one that has to go
> > in as several series, building up functionality as we go.
>
> I'll split it up as much as possible for next version.
>
> I was thinking of taking out the hot-reloading stuff for a follow-up
> series. In that case I would also add IIO_ACQUIRE_BUFFER_MODE().
> What do you think?
Sure, bring it in when needed. I just missread the code completely ;(
>
> >
> > I'll ignore all the DT stuff as sounds like that may radically change and
> > just take a fairly superficial first look at this.
>
> Yes, I will just address Krzysztof comments and leave that patch until
> we can discuss it with David.
>
> >
> > Jonathan
> >
>
> [...]
>
> >> +#include <linux/lockdep.h>
> >
> > Fairly unusual to see that header in a driver.
> > What's it here for?
>
> I included it for lockdep_assert_held().
Ah ok.
>
> [...]
>
> >> +/* IDACMAG constants */
> >> +#define ADS1262_IDACMAG_OFF 0
> >> +#define ADS1262_IDACMAG_COUNT 11
> >> +
> >> +/* REFMUX constants */
> >
> > Naming is good enough I'm not sure I'd bother with the comments
> > to say what these are.
> >
> > On option is to just group them with the register they are about
> > and using extra indenting to visually separate them from the register
> >
> > #define ADS1262_REFMUX_REG 0xxx
> > #define ADS1262_REFMUX_RMUXP_MASK GENMASK(5, 3)
> > #define ADS1262_RMUX_INTER 0
> > #define ADS1262_RMUX_AIN0_AIN1 1
> > #define ADS1262_RMUX_AIN2_AIN3 2
> > #define ADS1262_RMUX_AIN4_AIN5 3
> > #define ADS1262_RMUX_AVDD_AVSS 4
> > #define ADS1262_RMUX_COUNT 5
>
> I like this...
>
> > However, if you are going to have a terminating entry, an anonymous enum might be better
> > with that just as the last item.
>
> ...but this sounds good too. I'll go for what looks more organized.
>
> >
> > #define ADS1262_REFMUX_RMUXN_MASK GENMASK(2, 0)
> >
> >
> >> +#define ADS1262_RMUX_INTER 0
> >> +#define ADS1262_RMUX_AIN0_AIN1 1
> >> +#define ADS1262_RMUX_AIN2_AIN3 2
> >> +#define ADS1262_RMUX_AIN4_AIN5 3
> >> +#define ADS1262_RMUX_AVDD_AVSS 4
> >> +#define ADS1262_RMUX_COUNT 5
> >> +
> >> +struct ads1262_channel {
> >
> > As a general rule we tend to avoid bitfields because of all the problems
> > with how loose the C spec is on how these actually get laid out.
> > I'd just have this as a suitable 32 bit value and then have
> > defines for masks within that.
>
> Are you suggesting storing this whole struct data as a u32 and
> reading/writing with FIELD_*() helpers? I think that may be less
> readable but it would save memory. I don't know if I understood
> correctly though.
Yes. It's a pity that bitfields are loosely defined in C :(
>
> I'm dropping the bitfield approach for next version anyway.
>
> [...]
>
> >> +struct ads1262 {
> >> + struct spi_device *spi;
> >> + struct regmap *regmap;
> >> + struct iio_dev *indio_dev;
> >> + struct iio_trigger *trig;
> >> + struct gpio_desc *reset_gpiod;
> >> + struct gpio_desc *start_gpiod;
> >> +
> >> + void *scan_buffer;
> > I think this is always accessed as a __be32. If so just type it as that.
>
> I was hesitant to do that because of the space reserved at the end for
> the timestamp. Didn't feel right to assign __be32 when it would actually
> be something like
>
> struct {
> __be32 buff;
> aligned_s64 ts;
> };
>
> But I have no problem doing it.
I looked again. For this case we have some magic macros and generally
it's not worth the effort of making this dynamically sized unless you
have hundreds of channels. Here I would move it to the end of
struct ads1262 (so the _DMA_ part of this does the right thing) and use
IIO_DECLARE_DMA_BUFFER_WITH_TS(__be32, scan, DEFINE_FOR_MAX_CHANNELS);
That will declare a __be32 array but with the timestamp + padding etc.
> >> +static int ads1262_read_chip_name(struct ads1262 *st, char **name)
> >> +{
> >> + struct device *dev = &st->spi->dev;
> >> + u8 dev_id;
> >> + unsigned int val;
> >> + int ret;
> >> +
> >> + ret = regmap_read(st->regmap, ADS1262_ID_REG, &val);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + dev_id = FIELD_GET(ADS1262_DEV_ID_MASK, val);
> >> +
> >> + switch (dev_id) {
> >> + case ADS1262_DEV_ID_ADS1262:
> >> + *name = "ads1262";
> >
> > Given, at somepoint I would guess you'll want to support the auxiliary adc
> > on the 1263, I'd start with a struct chip_info (with the name in there)
> > and pick that rather than just the name here.
>
> Makes sense. In that case I can add a dev_warn if the name doesn't match
> the internal model. Would that be ok or would you prefer dev_dbg?
dev_info() probably as if we do see this it isn't supposed to be a problem
(unless DT is broken). Note there is an ongoing discussion with Kryzsztof about
what we should do for detectable parts like this. So my mental model
of this might not be where that ends up. (See the various versions of the SLF3S
flow sensor driver).
Jonathan
^ permalink raw reply
* Re: [PATCH 2/5] iio: adc: Add ti-ads1262 driver
From: Jonathan Cameron @ 2026-06-21 14:33 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Kurt Borja, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Bartosz Golaszewski, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, devicetree, linux-kernel, linux-gpio
In-Reply-To: <a994993e-7459-48a2-a8d7-823c1b0c9545@kernel.org>
On Mon, 15 Jun 2026 06:30:28 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 14/06/2026 22:56, Kurt Borja wrote:
> > On Sat Jun 13, 2026 at 1:59 PM -05, Krzysztof Kozlowski wrote:
> >
> > [...]
> >
> >> Functions used by probe() should be before probe(), not somewhere in the
> >> middle of the code. IOW, entire probe is together.
> >
> > I they all are, it's just that regmap stuff takes a huge chunk. I'll
> > check how to reorganize.
> >
> > [...]
> >
> >>> +static const struct of_device_id ads1262_of_match[] = {
> >>> + { .compatible = "ti,ads1262" },
> >>> + { .compatible = "ti,ads1263" },
> >>
> >> So devices are fully compatible? Then it should be expressed in the
> >> binding and drop one entry here.
> >
> > Not fully compatible as Jonathan said. One is a subset of the other.
>
> This is THE meaning of compatible!
This one I'm in agreement with. It is a strict subset, so should be
using a fallback. If the fallback is used, you just get support of the
stuff in the simpler chip (or if you can override it with a chip ID
you might still 'upgrade' to the more complex driver support).
If you do end up with properties that only apply to 'new' parts of
the more complex chip then they should be verified as part of the
binding (assuming you can do that without the verifier complaining
- I haven't checked!)
The SLF3F discussion is about (to me) less obvious case of not a strict
subset, but rather being detectable parts with different channel related
properties. In that case the ID match is necessary for anything to work.
Anyhow, that discussion is in a different thread and not really relevant
here.
Jonathan
>
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Jonathan Cameron @ 2026-06-21 14:33 UTC (permalink / raw)
To: Nuno Sá
Cc: Conor Dooley, Janani Sunil, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <ajVlD-j0nIGrRVow@nsa>
On Fri, 19 Jun 2026 16:54:11 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
> > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > > > >
> > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > >
> > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > integrated precision reference.
> > > > > > > > > ...
> > > > > > > > > Probably others may comment on that, but...
> > > > > > > > >
> > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > >
> > > > > > > > > That way I suppose that an example would look like...
> > > > > > > > > > +
> > > > > > > > > > +patternProperties:
> > > > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > + type: object
> > > > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > > > +
> > > > > > > > > > + properties:
> > > > > > > > > > + reg:
> > > > > > > > > > + description: Channel number.
> > > > > > > > > > + minimum: 0
> > > > > > > > > > + maximum: 15
> > > > > > > > > > +
> > > > > > > > > > + adi,output-range-microvolt:
> > > > > > > > > > + description: |
> > > > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > > > + oneOf:
> > > > > > > > > > + - items:
> > > > > > > > > > + - const: 0
> > > > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > + - items:
> > > > > > > > > > + - const: -5000000
> > > > > > > > > > + - const: 5000000
> > > > > > > > > > + - items:
> > > > > > > > > > + - const: -10000000
> > > > > > > > > > + - const: 10000000
> > > > > > > > > > + - items:
> > > > > > > > > > + - const: -15000000
> > > > > > > > > > + - const: 15000000
> > > > > > > > > > + - items:
> > > > > > > > > > + - const: -20000000
> > > > > > > > > > + - const: 20000000
> > > > > > > > > > +
> > > > > > > > > > + required:
> > > > > > > > > > + - reg
> > > > > > > > > > +
> > > > > > > > > > + additionalProperties: false
> > > > > > > > > > +
> > > > > > > > > > +required:
> > > > > > > > > > + - compatible
> > > > > > > > > > + - reg
> > > > > > > > > > + - vdd-supply
> > > > > > > > > > + - avdd-supply
> > > > > > > > > > + - hvdd-supply
> > > > > > > > > > +
> > > > > > > > > > +dependencies:
> > > > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > > > +
> > > > > > > > > > +allOf:
> > > > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > +
> > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > +
> > > > > > > > > > +examples:
> > > > > > > > > > + - |
> > > > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > +
> > > > > > > > > > + spi {
> > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > +
> > > > > > > > > > + dac@0 {
> > > > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > > > + reg = <0>;
> > > > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > > > +
> > > > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > > > +
> > > > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > +
> > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > +
> > > > > > > > > > + channel@0 {
> > > > > > > > > > + reg = <0>;
> > > > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > + };
> > > > > > > > > > +
> > > > > > > > > > + channel@1 {
> > > > > > > > > > + reg = <1>;
> > > > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > + };
> > > > > > > > > > +
> > > > > > > > > > + channel@2 {
> > > > > > > > > > + reg = <2>;
> > > > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > + };
> > > > > > > > > > + };
> > > > > > > > > > + };
> > > > > > > > > ...
> > > > > > > > >
> > > > > > > > > spi {
> > > > > > > > > #address-cells = <1>;
> > > > > > > > > #size-cells = <0>;
> > > > > > > > >
> > > > > > > > > multi-dac@0 {
> > > > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > > > reg = <0>;
> > > > > > > > > spi-max-frequency = <25000000>;
> > > > > > > > >
> > > > > > > > > #address-cells = <1>;
> > > > > > > > > #size-cells = <0>;
> > > > > > > > >
> > > > > > > > > dac@0 {
> > > > > > > > > reg = <0>;
> > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > >
> > > > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > >
> > > > > > > > > #address-cells = <1>;
> > > > > > > > > #size-cells = <0>;
> > > > > > > > >
> > > > > > > > > channel@0 {
> > > > > > > > > reg = <0>;
> > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > channel@1 {
> > > > > > > > > reg = <1>;
> > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > channel@2 {
> > > > > > > > > reg = <2>;
> > > > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > };
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > dac@1 {
> > > > > > > > > reg = <1>;
> > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > >
> > > > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > >
> > > > > > > > > #address-cells = <1>;
> > > > > > > > > #size-cells = <0>;
> > > > > > > > >
> > > > > > > > > channel@0 {
> > > > > > > > > reg = <0>;
> > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > channel@1 {
> > > > > > > > > reg = <1>;
> > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > };
> > > > > > > > > }
> > > > > > > > > };
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > then you might need something like:
> > > > > > > > >
> > > > > > > > > patternProperties:
> > > > > > > > > "^dac@[0-3]$":
> > > > > > > > >
> > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > >
> > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > >
> > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > > > Hi Rodrigo,
> > > > > > > >
> > > > > > > > Thank you for looking at this.
> > > > > > > >
> > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > speculatively without a validating use case.
> > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > >
> > > > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > in all devices in the chain as one big SPI message.
> > > > > > >
> > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > > > longer term how to support it cleanly in SPI.
> > > > >
> > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > see if I can find what I am thinking of...
> > > >
> > > >
> > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > slightly different properties.
> > > >
> > > > microchip,device-addr:
> > > > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > enum: [0, 1, 2, 3]
> > > > default: 0
> > > >
> > > > and
> > > >
> > > >
> > > > microchip,hw-device-address:
> > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > minimum: 0
> > > > maximum: 3
> > > > description:
> > > > The address is set on a per-device basis by fuses in the factory,
> > > > configured on request. If not requested, the fuses are set for 0x1.
> > > > The device address is part of the device markings to avoid
> > > > potential confusion. This address is coded on two bits, so four possible
> > > > addresses are available when multiple devices are present on the same
> > > > SPI bus with only one Chip Select line for all devices.
> > > > Each device communication starts by a CS falling edge, followed by the
> > > > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > which is first one on the wire).
> > > >
> > > > This sounds exactly like the sort of feature that you're dealing with
> > > > here?
> > > >
> > >
> > > The core idea yes but for this chip, things are a bit more annoying (but
> > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > have it's own supplies, pins and at the very least, channels with maybe
> > > different scales. That is why Janani is proposing dac nodes. Given I
> > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > about solving this at the spi level.
> > >
> > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > together in the same bus.
> >
> > I'm definitely missing something, because that property for the
> > microchip devices is not impacted what else is on the bus. AFAICT, you
> > could have an mcp3911 and an mcp3564 on the same bus even though both
> > are completely different devices with different drivers. They have
> > individual device nodes and their own supplies etc etc. These aren't
> > per-channel properties on an adc or dac, they're per child device on a
> > spi bus.
>
> Maybe I'm the one missing something :). IIRC, spi would not allow two
> devices on the same CS right? Because for this chip we would need
> something like:
>
> spi {
> dac@0 {
> reg = <0>;
> adi,pin-id = <0>;
> };
>
> dac@1 {
> reg = <0>; // which seems already problematic?
> adi,pin-id <1>;
> };
>
> ...
>
> //up to 4
> };
Yeah. It's not clear to me how that works for the microchip devices
(I suspect it doesn't!)
Just thinking as I type, but could we do something a bit nasty with
a gpio mux that doesn't actually switch but represents the GPIO being
shared? Given this is all tied to the spi bus that should all happen
under serializing locks.
Agreed though that this would be nicer as an SPI thing that let
us specify that a single CS is share by multiple devices and their
is some other signal acting to select which one we are talking to.
Jonathan
>
> - Nuno Sá
>
>
^ permalink raw reply
* [PATCH] of_numa: fix return -EINVAL when numa-node-id is not found in last node
From: Sang-Heon Jeon @ 2026-06-21 14:39 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan
Cc: Sang-Heon Jeon, Andrew Morton, Dan Williams, David Hildenbrand,
devicetree, Jonathan Cameron, Mike Rapoport (Microsoft)
When the numa-node-id property is not found in the last memory node,
of_property_read_u32() returns -EINVAL, which then becomes the return
value of of_numa_parse_memory_nodes(), even though earlier memory nodes
were parsed successfully.
Commit 7e488677a54a ("of, numa: return -EINVAL when no numa-node-id is
found") meant -EINVAL to be returned only when the numa-node-id property
is not found at all, not when it is found in an earlier memory node but
not in the last.
Check whether at least one memory node was parsed successfully, and return
0 in that case, -EINVAL otherwise, so the return value of
of_property_read_u32() for the last memory node no longer corrupts the
overall return value.
Also include other minor changes for readability improvement with no
functional change.
Fixes: 7e488677a54a ("of, numa: return -EINVAL when no numa-node-id is found")
Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
QEMU-based test results
DTB memory nodes defined in DT
- memory@80000000 (0x80000000-0x9fffffff) numa-node-id = <0>
- memory@c0000000 (0xc0000000-0xdfffffff) numa-node-id = <0>
- memory@e0000000 (0xe0000000-0xffffffff) numa-node-id = <1>
- memory@a0000000 (0xa0000000-0xbfffffff) (no numa-node-id)
1) AS-IS (before fix)
[ 0.000000] NUMA: Faking a node at [mem 0x0000000080000000-0x00000000ffffffff]
[ 0.069152] futex hash table entries: 512 (32768 bytes on 1 NUMA nodes, total 32 KiB, linear).
...
2) TO-BE (after fix)
[ 0.000000] NUMA: Node 0 [mem 0x80000000-0x9fffffff] + [mem 0xc0000000-0xdfffffff] -> [mem 0x80000000-0xdfffffff]
[ 0.000000] Initmem setup node 0 [mem 0x0000000080000000-0x00000000dfffffff]
[ 0.000000] Initmem setup node 1 [mem 0x00000000e0000000-0x00000000ffffffff]
[ 0.076854] futex hash table entries: 256 (16384 bytes on 2 NUMA nodes, total 32 KiB, linear).
...
Tested the scenarios below to confirm no regression, and all produced
the same result on AS-IS and TO-BE:
- all / no memory nodes tagged
- untagged memory node exist, but not last
- empty / out-of-range numa-node-id
- memory node without reg
---
drivers/of/of_numa.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
index cd2dc8e825c9..13f2f6d238c9 100644
--- a/drivers/of/of_numa.c
+++ b/drivers/of/of_numa.c
@@ -42,7 +42,8 @@ static int __init of_numa_parse_memory_nodes(void)
struct device_node *np = NULL;
struct resource rsrc;
u32 nid;
- int i, r = -EINVAL;
+ int i, r;
+ bool found = false;
for_each_node_by_type(np, "memory") {
r = of_property_read_u32(np, "numa-node-id", &nid);
@@ -53,26 +54,32 @@ static int __init of_numa_parse_memory_nodes(void)
* "numa-node-id" property
*/
continue;
+ if (r)
+ goto err;
if (nid >= MAX_NUMNODES) {
pr_warn("Node id %u exceeds maximum value\n", nid);
- r = -EINVAL;
+ goto err;
}
- for (i = 0; !r && !of_address_to_resource(np, i, &rsrc); i++) {
+ for (i = 0; !of_address_to_resource(np, i, &rsrc); i++) {
r = numa_add_memblk(nid, rsrc.start, rsrc.end + 1);
- if (!r)
- node_set(nid, numa_nodes_parsed);
+ if (r)
+ goto err;
+ node_set(nid, numa_nodes_parsed);
}
+ if (!i)
+ goto err;
- if (!i || r) {
- of_node_put(np);
- pr_err("bad property in memory node\n");
- return r ? : -EINVAL;
- }
+ found = true;
}
- return r;
+ return found ? 0 : -EINVAL;
+
+err:
+ of_node_put(np);
+ pr_err("bad property in memory node\n");
+ return r ?: -EINVAL;
}
static int __init of_numa_parse_distance_map_v1(struct device_node *map)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 5/5] iio: adc: Add ti-ads1263-adc2 driver
From: Jonathan Cameron @ 2026-06-21 14:41 UTC (permalink / raw)
To: Kurt Borja
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Linus Walleij,
Bartosz Golaszewski, David Lechner, Nuno Sá, Andy Shevchenko,
linux-iio, devicetree, linux-kernel, linux-gpio
In-Reply-To: <DJ92CLJIJ04T.3HSUHGGSF8EPG@gmail.com>
On Sun, 14 Jun 2026 15:43:49 -0500
"Kurt Borja" <kuurtb@gmail.com> wrote:
> On Sat Jun 13, 2026 at 9:10 AM -05, Jonathan Cameron wrote:
> > On Fri, 12 Jun 2026 17:46:23 -0500
> > Kurt Borja <kuurtb@gmail.com> wrote:
> >
> >> The TI ADS1263 includes an auxiliary, 24-bit, delta-sigma ADC (ADC2).
> >> ADC2 operation is independent of ADC1, with independent selections of
> >> input channel, reference voltage, sample rate, and channel gain
> >>
> >> Add support for this ADC as an independent IIO device, through the
> >> auxiliary bus API.
> >
> > A few things inline.
> >
> >>
> >> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> >
> >> diff --git a/drivers/iio/adc/ti-ads1262.c b/drivers/iio/adc/ti-ads1262.c
> >> index b33505e7fdc7..1a4b2f934d43 100644
> >> --- a/drivers/iio/adc/ti-ads1262.c
> >> +++ b/drivers/iio/adc/ti-ads1262.c
> >
> >> +static int ads1262_aux_device_setup(struct ads1262 *st)
> >> +{
> >> + struct device *dev = &st->spi->dev;
> >> + struct ads1263_adc2_channel *chans;
> >> + struct auxiliary_device *adev;
> >> + struct ads1263_adc2_ctx *ctx;
> >> + struct fwnode_handle *node;
> >> + int id, ret;
> >> +
> >> + node = device_get_named_child_node(dev, "adc");
> >> + if (!node)
> >> + return 0;
> >> +
> >> + ctx = kzalloc_obj(*ctx);
> >> + if (!ctx) {
> >> + ret = -ENOMEM;
> >> + goto out_node_put;
> >> + }
> >> +
> >> + id = ida_alloc(&ads1262_ida, GFP_KERNEL);
> >> + if (id < 0) {
> >> + ret = id;
> >> + goto out_free_adc2;
> >> + }
> >> +
> >> + chans = kcalloc(st->num_channels, sizeof(*chans), GFP_KERNEL);
> >> + if (!chans) {
> >> + ret = -ENOMEM;
> >> + goto out_free_id;
> >> + }
> >> +
> >> + for (unsigned int i = 0; i < st->num_channels; i++) {
> >> + chans[i].negative_input = st->channels[i].negative_input;
> >> + chans[i].positive_input = st->channels[i].positive_input;
> >> + }
> >> +
> >> + ctx->chip = st;
> >> + ctx->num_channels = st->num_channels;
> >> + ctx->channels = chans;
> >> + ctx->enable = ads1263_adc2_enable;
> >> + ctx->start = ads1263_adc2_start;
> >> + ctx->stop = ads1263_adc2_stop;
> >> + ctx->read = ads1263_adc2_read;
> >> + mutex_init(&ctx->chan_lock);
> > devm_mutex_init()
>
> I actually call mutex_destroy() on device .release.
>
> I think it makes more sense that way, otherwise we would UAF?
It does indeed make more sense there.
Whether it ends up as a UAF will depend on how the mutex is used.
I 'think' you are fine either way because it is always in an
IIO callback which depending on call path is ether from sysfs
removed in the devm_iio_device_unregister() path or gated on
the iio_dev->info being set to NULL for in kernel users.
Handing over lifetime ownership to the device is absolutely fine
and a bit easier to reason about so ok to leave it like this.
> > },
> >
> >> + { }
> >> +};
> >> +MODULE_DEVICE_TABLE(auxiliary, ads1263_adc2_auxiliary_match);
>
> Thanks for your feedback, Jonathan! Apologies if this version was a
> little rough... I'm a bit embarrased by the bugs found by Sashiko.
>
Lol. We all have that problem - sometimes it is irritatingly good
at spotting silly mistakes ;)
^ permalink raw reply
* [PATCH v2] dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
From: Aditya Chari @ 2026-06-21 9:19 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, gregkh
Cc: jacobsfeder, devicetree, linux-staging, linux-kernel,
Aditya Chari
In-Reply-To: <20260621085209.52651-1-adi25charis@gmail.com>
The axis-fifo driver's compatible strings were undocumented, flagged
by checkpatch.pl as UNDOCUMENTED_DT_STRING. Add a YAML devicetree
binding document for drivers/staging/axis-fifo, converted from and
replacing the existing free-form text binding (axis-fifo.txt), which
this patch removes.
Signed-off-by: Aditya Chari <adi25charis@gmail.com>
---
Changes since v1:
- Fixed xlnx,rx/tx-fifo-depth: depth is in 32-bit words, not bytes,
matching the driver's overflow check in axis_fifo_write() and the
wording of the original text binding.
- Restored the full set of hardware-generated properties (interrupt-
names, AXI-Stream protocol/width properties, has-axis-t* feature
flags, fifo threshold properties, etc.) so that additionalProperties:
false does not reject valid device trees generated for real hardware.
- Removed the now-superseded axis-fifo.txt text binding.
.../bindings/misc/xlnx,axi-fifo-mm-s.yaml | 221 ++++++++++++++++++
drivers/staging/axis-fifo/axis-fifo.txt | 96 --------
2 files changed, 221 insertions(+), 96 deletions(-)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
delete mode 100644 drivers/staging/axis-fifo/axis-fifo.txt
diff --git a/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
new file mode 100644
index 000000000..f4606b13c
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
@@ -0,0 +1,221 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/xlnx,axi-fifo-mm-s.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx AXI-Stream FIFO (axis-fifo)
+
+maintainers:
+ - Jacob Feder <jacobsfeder@gmail.com>
+
+description:
+ The AXI-Stream FIFO (AXIS-FIFO) IP core provides a memory-mapped AXI4-Lite
+ interface for sending and receiving data over an AXI4-Stream interface
+ using FIFO buffers. Currently supports only store-forward mode with a
+ 32-bit AXI4-Lite interface; cut-through mode and full AXI4 are not
+ supported. See Xilinx PG080 for IP details.
+
+properties:
+ compatible:
+ enum:
+ - xlnx,axi-fifo-mm-s-4.1
+ - xlnx,axi-fifo-mm-s-4.2
+ - xlnx,axi-fifo-mm-s-4.3
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ items:
+ - const: interrupt
+
+ xlnx,axi-str-rxd-tdata-width:
+ description:
+ Width in bits of the AXI4-Stream receive data interface. Only a
+ width of 32 is currently supported by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 32
+
+ xlnx,axi-str-txd-tdata-width:
+ description:
+ Width in bits of the AXI4-Stream transmit data interface. Only a
+ width of 32 is currently supported by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 32
+
+ xlnx,axi-str-txc-tdata-width:
+ description:
+ Width in bits of the AXI4-Stream transmit control interface.
+ Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,axi-str-rxd-protocol:
+ description: AXI-Stream receive data protocol. Ignored by the driver.
+ enum: [ XIL_AXI_STREAM_ETH_DATA ]
+
+ xlnx,axi-str-txd-protocol:
+ description: AXI-Stream transmit data protocol. Ignored by the driver.
+ enum: [ XIL_AXI_STREAM_ETH_DATA ]
+
+ xlnx,axi-str-txc-protocol:
+ description: AXI-Stream transmit control protocol. Ignored by the driver.
+ enum: [ XIL_AXI_STREAM_ETH_CTRL ]
+
+ xlnx,axis-tdest-width:
+ description: AXI-Stream TDEST width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,axis-tid-width:
+ description: AXI-Stream TID width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,axis-tuser-width:
+ description: AXI-Stream TUSER width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,data-interface-type:
+ description: Data interface type. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,has-axis-tdest:
+ description:
+ Whether the AXI-Stream interface has TDEST. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,has-axis-tid:
+ description:
+ Whether the AXI-Stream interface has TID. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,has-axis-tkeep:
+ description:
+ Whether the AXI-Stream interface has TKEEP. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,has-axis-tstrb:
+ description:
+ Whether the AXI-Stream interface has TSTRB. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,has-axis-tuser:
+ description:
+ Whether the AXI-Stream interface has TUSER. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,rx-fifo-depth:
+ description:
+ Depth in 32-bit words of the receive FIFO, as configured in the
+ IP core.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,rx-fifo-pe-threshold:
+ description: RX programmable empty interrupt threshold. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,rx-fifo-pf-threshold:
+ description: RX programmable full interrupt threshold. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,s-axi-id-width:
+ description: AXI4-Lite ID width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,s-axi4-data-width:
+ description: AXI4-Lite data width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,select-xpm:
+ description: Whether XPM macros are used. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,tx-fifo-depth:
+ description:
+ Depth in 32-bit words of the transmit FIFO, as configured in the
+ IP core.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,tx-fifo-pe-threshold:
+ description: TX programmable empty interrupt threshold. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,tx-fifo-pf-threshold:
+ description: TX programmable full interrupt threshold. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,use-rx-cut-through:
+ description:
+ Whether RX cut-through mode is used. Not supported by the driver
+ and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,use-rx-data:
+ description:
+ Indicates whether the receive data path is present, as configured
+ in the IP core. A value of 1 enables the receive path, 0 disables it.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+
+ xlnx,use-tx-ctrl:
+ description:
+ Whether the transmit control interface is used. Not supported by
+ the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,use-tx-cut-through:
+ description:
+ Whether TX cut-through mode is used. Not supported by the driver
+ and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,use-tx-data:
+ description:
+ Indicates whether the transmit data path is present, as configured
+ in the IP core. A value of 1 enables the transmit path, 0 disables it.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - xlnx,axi-str-rxd-tdata-width
+ - xlnx,axi-str-txd-tdata-width
+ - xlnx,rx-fifo-depth
+ - xlnx,tx-fifo-depth
+ - xlnx,use-rx-data
+ - xlnx,use-tx-data
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ axi_fifo_mm_s_0: axi-fifo-mm-s@40000000 {
+ compatible = "xlnx,axi-fifo-mm-s-4.1";
+ reg = <0x40000000 0x10000>;
+ interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>;
+ xlnx,axi-str-rxd-tdata-width = <32>;
+ xlnx,axi-str-txd-tdata-width = <32>;
+ xlnx,rx-fifo-depth = <512>;
+ xlnx,tx-fifo-depth = <32768>;
+ xlnx,use-rx-data = <1>;
+ xlnx,use-tx-data = <1>;
+ };
diff --git a/drivers/staging/axis-fifo/axis-fifo.txt b/drivers/staging/axis-fifo/axis-fifo.txt
deleted file mode 100644
index 413b81a53..000000000
--- a/drivers/staging/axis-fifo/axis-fifo.txt
+++ /dev/null
@@ -1,96 +0,0 @@
-Xilinx AXI-Stream FIFO v4.1 IP core
-
-This IP core has read and write AXI-Stream FIFOs, the contents of which can
-be accessed from the AXI4 memory-mapped interface. This is useful for
-transferring data from a processor into the FPGA fabric. The driver creates
-a character device that can be read/written to with standard
-open/read/write/close.
-
-See Xilinx PG080 document for IP details.
-
-Currently supports only store-forward mode with a 32-bit
-AXI4-Lite interface. DOES NOT support:
- - cut-through mode
- - AXI4 (non-lite)
-
-Required properties:
-- compatible: Should be one of:
- "xlnx,axi-fifo-mm-s-4.1"
- "xlnx,axi-fifo-mm-s-4.2"
- "xlnx,axi-fifo-mm-s-4.3"
-- interrupt-names: Should be "interrupt"
-- interrupt-parent: Should be <&intc>
-- interrupts: Should contain interrupts lines.
-- reg: Should contain registers location and length.
-- xlnx,axi-str-rxd-protocol: Should be "XIL_AXI_STREAM_ETH_DATA"
-- xlnx,axi-str-rxd-tdata-width: Should be <0x20>
-- xlnx,axi-str-txc-protocol: Should be "XIL_AXI_STREAM_ETH_CTRL"
-- xlnx,axi-str-txc-tdata-width: Should be <0x20>
-- xlnx,axi-str-txd-protocol: Should be "XIL_AXI_STREAM_ETH_DATA"
-- xlnx,axi-str-txd-tdata-width: Should be <0x20>
-- xlnx,axis-tdest-width: AXI-Stream TDEST width (ignored by the driver)
-- xlnx,axis-tid-width: AXI-Stream TID width (ignored by the driver)
-- xlnx,axis-tuser-width: AXI-Stream TUSER width (ignored by the driver)
-- xlnx,data-interface-type: Should be <0x0> (ignored by the driver)
-- xlnx,has-axis-tdest: Should be <0x0> (this feature isn't supported)
-- xlnx,has-axis-tid: Should be <0x0> (this feature isn't supported)
-- xlnx,has-axis-tkeep: Should be <0x0> (this feature isn't supported)
-- xlnx,has-axis-tstrb: Should be <0x0> (this feature isn't supported)
-- xlnx,has-axis-tuser: Should be <0x0> (this feature isn't supported)
-- xlnx,rx-fifo-depth: Depth of RX FIFO in words
-- xlnx,rx-fifo-pe-threshold: RX programmable empty interrupt threshold
- (ignored by the driver)
-- xlnx,rx-fifo-pf-threshold: RX programmable full interrupt threshold
- (ignored by the driver)
-- xlnx,s-axi-id-width: Should be <0x4> (ignored by the driver)
-- xlnx,s-axi4-data-width: Should be <0x20> (ignored by the driver)
-- xlnx,select-xpm: Should be <0x0> (ignored by the driver)
-- xlnx,tx-fifo-depth: Depth of TX FIFO in words
-- xlnx,tx-fifo-pe-threshold: TX programmable empty interrupt threshold
- (ignored by the driver)
-- xlnx,tx-fifo-pf-threshold: TX programmable full interrupt threshold
- (ignored by the driver)
-- xlnx,use-rx-cut-through: Should be <0x0> (this feature isn't supported)
-- xlnx,use-rx-data: <0x1> if RX FIFO is enabled, <0x0> otherwise
-- xlnx,use-tx-ctrl: Should be <0x0> (this feature isn't supported)
-- xlnx,use-tx-cut-through: Should be <0x0> (this feature isn't supported)
-- xlnx,use-tx-data: <0x1> if TX FIFO is enabled, <0x0> otherwise
-
-Example:
-
-axi_fifo_mm_s_0: axi_fifo_mm_s@43c00000 {
- compatible = "xlnx,axi-fifo-mm-s-4.1";
- interrupt-names = "interrupt";
- interrupt-parent = <&intc>;
- interrupts = <0 29 4>;
- reg = <0x43c00000 0x10000>;
- xlnx,axi-str-rxd-protocol = "XIL_AXI_STREAM_ETH_DATA";
- xlnx,axi-str-rxd-tdata-width = <0x20>;
- xlnx,axi-str-txc-protocol = "XIL_AXI_STREAM_ETH_CTRL";
- xlnx,axi-str-txc-tdata-width = <0x20>;
- xlnx,axi-str-txd-protocol = "XIL_AXI_STREAM_ETH_DATA";
- xlnx,axi-str-txd-tdata-width = <0x20>;
- xlnx,axis-tdest-width = <0x4>;
- xlnx,axis-tid-width = <0x4>;
- xlnx,axis-tuser-width = <0x4>;
- xlnx,data-interface-type = <0x0>;
- xlnx,has-axis-tdest = <0x0>;
- xlnx,has-axis-tid = <0x0>;
- xlnx,has-axis-tkeep = <0x0>;
- xlnx,has-axis-tstrb = <0x0>;
- xlnx,has-axis-tuser = <0x0>;
- xlnx,rx-fifo-depth = <0x200>;
- xlnx,rx-fifo-pe-threshold = <0x2>;
- xlnx,rx-fifo-pf-threshold = <0x1fb>;
- xlnx,s-axi-id-width = <0x4>;
- xlnx,s-axi4-data-width = <0x20>;
- xlnx,select-xpm = <0x0>;
- xlnx,tx-fifo-depth = <0x8000>;
- xlnx,tx-fifo-pe-threshold = <0x200>;
- xlnx,tx-fifo-pf-threshold = <0x7ffb>;
- xlnx,use-rx-cut-through = <0x0>;
- xlnx,use-rx-data = <0x0>;
- xlnx,use-tx-ctrl = <0x0>;
- xlnx,use-tx-cut-through = <0x0>;
- xlnx,use-tx-data = <0x1>;
-};
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2] dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
From: sashiko-bot @ 2026-06-21 15:34 UTC (permalink / raw)
To: Aditya Chari; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260621091953.53281-1-adi25charis@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Low] Missing `$ref` type definition for custom string properties in DT binding.
Pre-existing issues:
- [Critical] Use-After-Free on device unbind due to lack of synchronization with open file descriptors.
- [High] Integer underflow in bounds check allows bypassing transmit packet size restriction.
--
commit 97e86b437f0cf66fc60eec47c4742a54db8053f8
Author: Aditya Chari <adi25charis@gmail.com>
dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
The axis-fifo driver's compatible strings were undocumented, flagged
by checkpatch.pl as UNDOCUMENTED_DT_STRING. Add a YAML devicetree
binding document for drivers/staging/axis-fifo, converted from and
replacing the existing free-form text binding (axis-fifo.txt), which
this patch removes.
> diff --git a/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
> @@ -0,0 +1,221 @@
[ ... ]
> + xlnx,axi-str-rxd-protocol:
> + description: AXI-Stream receive data protocol. Ignored by the driver.
> + enum: [ XIL_AXI_STREAM_ETH_DATA ]
> +
> + xlnx,axi-str-txd-protocol:
> + description: AXI-Stream transmit data protocol. Ignored by the driver.
> + enum: [ XIL_AXI_STREAM_ETH_DATA ]
> +
> + xlnx,axi-str-txc-protocol:
> + description: AXI-Stream transmit control protocol. Ignored by the driver.
> + enum: [ XIL_AXI_STREAM_ETH_CTRL ]
[Severity: Low]
Do these custom string properties need a $ref to types.yaml?
Device tree schema validation (dt_binding_check) requires custom properties
to explicitly specify their type. Providing an enum of strings might not be
sufficient for dt-schema to infer the type, which could cause type missing
or unknown schema validation errors.
Should these properties include a $ref: /schemas/types.yaml#/definitions/string
constraint?
[ ... ]
> + xlnx,tx-fifo-depth:
> + description:
> + Depth in 32-bit words of the transmit FIFO, as configured in the
> + IP core.
> + $ref: /schemas/types.yaml#/definitions/uint32
[Severity: High]
This isn't a bug introduced by this patch, but should we add a minimum: 4
constraint here to protect against an integer underflow in the driver?
In drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_write(), the bounds check
assumes tx_fifo_depth is at least 4:
if (!words_to_write || (len % sizeof(u32)) ||
(words_to_write > (fifo->tx_fifo_depth - 4)))
return -EINVAL;
If the device tree configures xlnx,tx-fifo-depth to a value less than 4, this
subtraction underflows to a massive unsigned integer, bypassing the bounds
check. This could allow an oversized packet to trigger a Transmit Packet
Overrun Error in the hardware.
[ ... ]
[Severity: Critical]
This is a pre-existing issue in the driver documented by this binding, but is
there a use-after-free vulnerability during device unbind?
The driver allocates its core state using devm_kzalloc():
drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_probe() {
...
fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
...
}
During removal, it deregisters the misc device but doesn't wait for active
VFS file references to be dropped:
drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_remove() {
...
misc_deregister(&fifo->miscdev);
...
}
When axis_fifo_remove() returns, the devres layer automatically frees the
axis_fifo structure. If a user process holds an open file descriptor and
subsequently calls write() or read(), it will dereference the freed pointer:
drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_write() {
struct axis_fifo *fifo = f->private_data;
...
}
Should the driver use a different allocation and reference counting strategy
to ensure the memory remains valid as long as the file descriptor is open?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260621091953.53281-1-adi25charis@gmail.com?part=1
^ permalink raw reply
* [PATCH v3] dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
From: Aditya Chari @ 2026-06-21 9:43 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, gregkh
Cc: jacobsfeder, devicetree, linux-staging, linux-kernel,
Aditya Chari
In-Reply-To: <20260621085209.52651-1-adi25charis@gmail.com>
The axis-fifo driver's compatible strings were undocumented, flagged
by checkpatch.pl as UNDOCUMENTED_DT_STRING. Add a YAML devicetree
binding document for drivers/staging/axis-fifo, converted from and
replacing the existing free-form text binding (axis-fifo.txt), which
this patch removes.
Constrain xlnx,tx-fifo-depth to a minimum of 4, since the driver
subtracts 4 from this value in its transmit bounds check and a
smaller value would underflow that check.
Signed-off-by: Aditya Chari <adi25charis@gmail.com>
---
Changes since v2:
- Added $ref: /schemas/types.yaml#/definitions/string to the three
AXI-Stream protocol enum properties (xlnx,axi-str-rxd-protocol,
xlnx,axi-str-txd-protocol, xlnx,axi-str-txc-protocol) for explicit
type consistency with the rest of the schema.
- Added minimum: 4 to xlnx,tx-fifo-depth, since the driver subtracts
4 from this value in its transmit bounds check
(axis_fifo_write()) and a smaller configured value would underflow
that unsigned check, bypassing the oversized-packet guard.
Changes since v1:
- Fixed xlnx,rx/tx-fifo-depth: depth is in 32-bit words, not bytes,
matching the driver's overflow check in axis_fifo_write() and the
wording of the original text binding.
- Restored the full set of hardware-generated properties (interrupt-
names, AXI-Stream protocol/width properties, has-axis-t* feature
flags, fifo threshold properties, etc.) so that additionalProperties:
false does not reject valid device trees generated for real hardware.
- Removed the now-superseded axis-fifo.txt text binding.
.../bindings/misc/xlnx,axi-fifo-mm-s.yaml | 227 ++++++++++++++++++
drivers/staging/axis-fifo/axis-fifo.txt | 96 --------
2 files changed, 227 insertions(+), 96 deletions(-)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
delete mode 100644 drivers/staging/axis-fifo/axis-fifo.txt
diff --git a/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
new file mode 100644
index 000000000..53bf52524
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
@@ -0,0 +1,227 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/xlnx,axi-fifo-mm-s.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx AXI-Stream FIFO (axis-fifo)
+
+maintainers:
+ - Jacob Feder <jacobsfeder@gmail.com>
+
+description:
+ The AXI-Stream FIFO (AXIS-FIFO) IP core provides a memory-mapped AXI4-Lite
+ interface for sending and receiving data over an AXI4-Stream interface
+ using FIFO buffers. Currently supports only store-forward mode with a
+ 32-bit AXI4-Lite interface; cut-through mode and full AXI4 are not
+ supported. See Xilinx PG080 for IP details.
+
+properties:
+ compatible:
+ enum:
+ - xlnx,axi-fifo-mm-s-4.1
+ - xlnx,axi-fifo-mm-s-4.2
+ - xlnx,axi-fifo-mm-s-4.3
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ items:
+ - const: interrupt
+
+ xlnx,axi-str-rxd-tdata-width:
+ description:
+ Width in bits of the AXI4-Stream receive data interface. Only a
+ width of 32 is currently supported by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 32
+
+ xlnx,axi-str-txd-tdata-width:
+ description:
+ Width in bits of the AXI4-Stream transmit data interface. Only a
+ width of 32 is currently supported by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 32
+
+ xlnx,axi-str-txc-tdata-width:
+ description:
+ Width in bits of the AXI4-Stream transmit control interface.
+ Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,axi-str-rxd-protocol:
+ description: AXI-Stream receive data protocol. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ XIL_AXI_STREAM_ETH_DATA ]
+
+ xlnx,axi-str-txd-protocol:
+ description: AXI-Stream transmit data protocol. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ XIL_AXI_STREAM_ETH_DATA ]
+
+ xlnx,axi-str-txc-protocol:
+ description: AXI-Stream transmit control protocol. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ XIL_AXI_STREAM_ETH_CTRL ]
+
+ xlnx,axis-tdest-width:
+ description: AXI-Stream TDEST width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,axis-tid-width:
+ description: AXI-Stream TID width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,axis-tuser-width:
+ description: AXI-Stream TUSER width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,data-interface-type:
+ description: Data interface type. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,has-axis-tdest:
+ description:
+ Whether the AXI-Stream interface has TDEST. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,has-axis-tid:
+ description:
+ Whether the AXI-Stream interface has TID. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,has-axis-tkeep:
+ description:
+ Whether the AXI-Stream interface has TKEEP. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,has-axis-tstrb:
+ description:
+ Whether the AXI-Stream interface has TSTRB. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,has-axis-tuser:
+ description:
+ Whether the AXI-Stream interface has TUSER. This feature is not
+ supported by the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,rx-fifo-depth:
+ description:
+ Depth in 32-bit words of the receive FIFO, as configured in the
+ IP core.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,rx-fifo-pe-threshold:
+ description: RX programmable empty interrupt threshold. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,rx-fifo-pf-threshold:
+ description: RX programmable full interrupt threshold. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,s-axi-id-width:
+ description: AXI4-Lite ID width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,s-axi4-data-width:
+ description: AXI4-Lite data width. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,select-xpm:
+ description: Whether XPM macros are used. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,tx-fifo-depth:
+ description:
+ Depth in 32-bit words of the transmit FIFO, as configured in the
+ IP core. Must be at least 4; the driver subtracts 4 from this
+ value when bounds-checking transmit writes, and a smaller value
+ underflows that check.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 4
+
+ xlnx,tx-fifo-pe-threshold:
+ description: TX programmable empty interrupt threshold. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,tx-fifo-pf-threshold:
+ description: TX programmable full interrupt threshold. Ignored by the driver.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xlnx,use-rx-cut-through:
+ description:
+ Whether RX cut-through mode is used. Not supported by the driver
+ and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,use-rx-data:
+ description:
+ Indicates whether the receive data path is present, as configured
+ in the IP core. A value of 1 enables the receive path, 0 disables it.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+
+ xlnx,use-tx-ctrl:
+ description:
+ Whether the transmit control interface is used. Not supported by
+ the driver and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,use-tx-cut-through:
+ description:
+ Whether TX cut-through mode is used. Not supported by the driver
+ and must be 0.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 0
+
+ xlnx,use-tx-data:
+ description:
+ Indicates whether the transmit data path is present, as configured
+ in the IP core. A value of 1 enables the transmit path, 0 disables it.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - xlnx,axi-str-rxd-tdata-width
+ - xlnx,axi-str-txd-tdata-width
+ - xlnx,rx-fifo-depth
+ - xlnx,tx-fifo-depth
+ - xlnx,use-rx-data
+ - xlnx,use-tx-data
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ axi_fifo_mm_s_0: axi-fifo-mm-s@40000000 {
+ compatible = "xlnx,axi-fifo-mm-s-4.1";
+ reg = <0x40000000 0x10000>;
+ interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>;
+ xlnx,axi-str-rxd-tdata-width = <32>;
+ xlnx,axi-str-txd-tdata-width = <32>;
+ xlnx,rx-fifo-depth = <512>;
+ xlnx,tx-fifo-depth = <32768>;
+ xlnx,use-rx-data = <1>;
+ xlnx,use-tx-data = <1>;
+ };
diff --git a/drivers/staging/axis-fifo/axis-fifo.txt b/drivers/staging/axis-fifo/axis-fifo.txt
deleted file mode 100644
index 413b81a53..000000000
--- a/drivers/staging/axis-fifo/axis-fifo.txt
+++ /dev/null
@@ -1,96 +0,0 @@
-Xilinx AXI-Stream FIFO v4.1 IP core
-
-This IP core has read and write AXI-Stream FIFOs, the contents of which can
-be accessed from the AXI4 memory-mapped interface. This is useful for
-transferring data from a processor into the FPGA fabric. The driver creates
-a character device that can be read/written to with standard
-open/read/write/close.
-
-See Xilinx PG080 document for IP details.
-
-Currently supports only store-forward mode with a 32-bit
-AXI4-Lite interface. DOES NOT support:
- - cut-through mode
- - AXI4 (non-lite)
-
-Required properties:
-- compatible: Should be one of:
- "xlnx,axi-fifo-mm-s-4.1"
- "xlnx,axi-fifo-mm-s-4.2"
- "xlnx,axi-fifo-mm-s-4.3"
-- interrupt-names: Should be "interrupt"
-- interrupt-parent: Should be <&intc>
-- interrupts: Should contain interrupts lines.
-- reg: Should contain registers location and length.
-- xlnx,axi-str-rxd-protocol: Should be "XIL_AXI_STREAM_ETH_DATA"
-- xlnx,axi-str-rxd-tdata-width: Should be <0x20>
-- xlnx,axi-str-txc-protocol: Should be "XIL_AXI_STREAM_ETH_CTRL"
-- xlnx,axi-str-txc-tdata-width: Should be <0x20>
-- xlnx,axi-str-txd-protocol: Should be "XIL_AXI_STREAM_ETH_DATA"
-- xlnx,axi-str-txd-tdata-width: Should be <0x20>
-- xlnx,axis-tdest-width: AXI-Stream TDEST width (ignored by the driver)
-- xlnx,axis-tid-width: AXI-Stream TID width (ignored by the driver)
-- xlnx,axis-tuser-width: AXI-Stream TUSER width (ignored by the driver)
-- xlnx,data-interface-type: Should be <0x0> (ignored by the driver)
-- xlnx,has-axis-tdest: Should be <0x0> (this feature isn't supported)
-- xlnx,has-axis-tid: Should be <0x0> (this feature isn't supported)
-- xlnx,has-axis-tkeep: Should be <0x0> (this feature isn't supported)
-- xlnx,has-axis-tstrb: Should be <0x0> (this feature isn't supported)
-- xlnx,has-axis-tuser: Should be <0x0> (this feature isn't supported)
-- xlnx,rx-fifo-depth: Depth of RX FIFO in words
-- xlnx,rx-fifo-pe-threshold: RX programmable empty interrupt threshold
- (ignored by the driver)
-- xlnx,rx-fifo-pf-threshold: RX programmable full interrupt threshold
- (ignored by the driver)
-- xlnx,s-axi-id-width: Should be <0x4> (ignored by the driver)
-- xlnx,s-axi4-data-width: Should be <0x20> (ignored by the driver)
-- xlnx,select-xpm: Should be <0x0> (ignored by the driver)
-- xlnx,tx-fifo-depth: Depth of TX FIFO in words
-- xlnx,tx-fifo-pe-threshold: TX programmable empty interrupt threshold
- (ignored by the driver)
-- xlnx,tx-fifo-pf-threshold: TX programmable full interrupt threshold
- (ignored by the driver)
-- xlnx,use-rx-cut-through: Should be <0x0> (this feature isn't supported)
-- xlnx,use-rx-data: <0x1> if RX FIFO is enabled, <0x0> otherwise
-- xlnx,use-tx-ctrl: Should be <0x0> (this feature isn't supported)
-- xlnx,use-tx-cut-through: Should be <0x0> (this feature isn't supported)
-- xlnx,use-tx-data: <0x1> if TX FIFO is enabled, <0x0> otherwise
-
-Example:
-
-axi_fifo_mm_s_0: axi_fifo_mm_s@43c00000 {
- compatible = "xlnx,axi-fifo-mm-s-4.1";
- interrupt-names = "interrupt";
- interrupt-parent = <&intc>;
- interrupts = <0 29 4>;
- reg = <0x43c00000 0x10000>;
- xlnx,axi-str-rxd-protocol = "XIL_AXI_STREAM_ETH_DATA";
- xlnx,axi-str-rxd-tdata-width = <0x20>;
- xlnx,axi-str-txc-protocol = "XIL_AXI_STREAM_ETH_CTRL";
- xlnx,axi-str-txc-tdata-width = <0x20>;
- xlnx,axi-str-txd-protocol = "XIL_AXI_STREAM_ETH_DATA";
- xlnx,axi-str-txd-tdata-width = <0x20>;
- xlnx,axis-tdest-width = <0x4>;
- xlnx,axis-tid-width = <0x4>;
- xlnx,axis-tuser-width = <0x4>;
- xlnx,data-interface-type = <0x0>;
- xlnx,has-axis-tdest = <0x0>;
- xlnx,has-axis-tid = <0x0>;
- xlnx,has-axis-tkeep = <0x0>;
- xlnx,has-axis-tstrb = <0x0>;
- xlnx,has-axis-tuser = <0x0>;
- xlnx,rx-fifo-depth = <0x200>;
- xlnx,rx-fifo-pe-threshold = <0x2>;
- xlnx,rx-fifo-pf-threshold = <0x1fb>;
- xlnx,s-axi-id-width = <0x4>;
- xlnx,s-axi4-data-width = <0x20>;
- xlnx,select-xpm = <0x0>;
- xlnx,tx-fifo-depth = <0x8000>;
- xlnx,tx-fifo-pe-threshold = <0x200>;
- xlnx,tx-fifo-pf-threshold = <0x7ffb>;
- xlnx,use-rx-cut-through = <0x0>;
- xlnx,use-rx-data = <0x0>;
- xlnx,use-tx-ctrl = <0x0>;
- xlnx,use-tx-cut-through = <0x0>;
- xlnx,use-tx-data = <0x1>;
-};
--
2.53.0
^ permalink raw reply related
* [PATCH] dt-bindings: clock: ti,clockdomain: Convert to DT schema
From: Bhargav Joshi @ 2026-06-21 15:53 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tero Kristo
Cc: linux-clk, devicetree, linux-kernel, goledhruva, m-chawdhry,
daniel.baluta, simona.toaca, j.bhargav.u
Convert TI clockdomain to yaml DT schema. Drop '#clock-cells' from the
required list as this binding doesn't define a new clock binding type,
it is used to group existing clock nodes under hardware hierarchy. Most
existing dts omit '#clock-cells'.
Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
---
.../devicetree/bindings/clock/ti/clockdomain.txt | 25 -------------
.../bindings/clock/ti/ti,clockdomain.yaml | 41 ++++++++++++++++++++++
2 files changed, 41 insertions(+), 25 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/ti/clockdomain.txt b/Documentation/devicetree/bindings/clock/ti/clockdomain.txt
deleted file mode 100644
index edf0b5d42768..000000000000
--- a/Documentation/devicetree/bindings/clock/ti/clockdomain.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-Binding for Texas Instruments clockdomain.
-
-This binding uses the common clock binding[1] in consumer role.
-Every clock on TI SoC belongs to one clockdomain, but software
-only needs this information for specific clocks which require
-their parent clockdomain to be controlled when the clock is
-enabled/disabled. This binding doesn't define a new clock
-binding type, it is used to group existing clock nodes under
-hardware hierarchy.
-
-[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Required properties:
-- compatible : shall be "ti,clockdomain"
-- #clock-cells : from common clock binding; shall be set to 0.
-- clocks : link phandles of clocks within this domain
-
-Optional properties:
-- clock-output-names : from common clock binding.
-
-Examples:
- dss_clkdm: dss_clkdm {
- compatible = "ti,clockdomain";
- clocks = <&dss1_alwon_fck_3430es2>, <&dss_ick_3430es2>;
- };
diff --git a/Documentation/devicetree/bindings/clock/ti/ti,clockdomain.yaml b/Documentation/devicetree/bindings/clock/ti/ti,clockdomain.yaml
new file mode 100644
index 000000000000..9494cbb1a942
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti/ti,clockdomain.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/ti/ti,clockdomain.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments clockdomain
+
+maintainers:
+ - Tero Kristo <kristo@kernel.org>
+
+description:
+ This binding uses the common clock binding in consumer role. Every clock on TI
+ SoC belongs to one clockdomain, but software only needs this information for
+ specific clocks which require their parent clockdomain to be controlled when
+ the clock is enabled/disabled. This binding doesn't define a new clock binding
+ type, it is used to group existing clock nodes under hardware hierarchy.
+
+properties:
+ compatible:
+ const: ti,clockdomain
+
+ "#clock-cells":
+ const: 0
+
+ clocks: true
+
+ clock-output-names: true
+
+required:
+ - compatible
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ dss_clkdm {
+ compatible = "ti,clockdomain";
+ clocks = <&dss1_alwon_fck_3430es2>, <&dss_ick_3430es2>;
+ };
---
base-commit: acb7500801e98639f6d8c2d796ed9f64cba83d3a
change-id: 20260610-ti-clockdomain-a27dd0fa1ad5
Best regards,
--
Bhargav
^ permalink raw reply related
* Re: [PATCH v3] dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
From: sashiko-bot @ 2026-06-21 15:54 UTC (permalink / raw)
To: Aditya Chari; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260621094312.53655-1-adi25charis@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing runtime bounds check for tx_fifo_depth causes integer underflow and potential DoS
- [High] Unvalidated hardware read can cause massive busy loop and CPU soft lockup
- [High] Hardware interrupts enabled before IRQ handler registration
--
commit cd9dfa74e4b12151a534638f2622f83e05f40fee
Author: Aditya Chari <adi25charis@gmail.com>
dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
This patch converts the free-form text binding for the Xilinx AXI-Stream
FIFO IP core into a YAML schema. It also enforces a minimum value of 4
for the xlnx,tx-fifo-depth property to match the driver's transmit bounds
check.
> diff --git a/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
> new file mode 100644
> index 0000000000000..53bf52524e74d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
> @@ -0,0 +1,227 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/xlnx,axi-fifo-mm-s.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
[ ... ]
> +properties:
> + compatible:
> + enum:
> + - xlnx,axi-fifo-mm-s-4.1
> + - xlnx,axi-fifo-mm-s-4.2
> + - xlnx,axi-fifo-mm-s-4.3
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
[Severity: High]
This is a pre-existing issue, but does the driver enable interrupts before
registering the IRQ handler?
Looking at drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_probe():
reset_ip_core(fifo);
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return irq;
rc = devm_request_irq(fifo->dt_device, irq, &axis_fifo_irq, 0,
DRIVER_NAME, fifo);
And in reset_ip_core():
iowrite32(XLLF_INT_TC_MASK | XLLF_INT_RC_MASK | XLLF_INT_RPURE_MASK |
XLLF_INT_RPORE_MASK | XLLF_INT_RPUE_MASK |
XLLF_INT_TPOE_MASK | XLLF_INT_TSE_MASK,
fifo->base_addr + XLLF_IER_OFFSET);
If the hardware immediately asserts an interrupt, could this trigger a spurious
interrupt before devm_request_irq() completes, permanently disabling the IRQ
line?
[ ... ]
> + xlnx,tx-fifo-depth:
> + description:
> + Depth in 32-bit words of the transmit FIFO, as configured in the
> + IP core. Must be at least 4; the driver subtracts 4 from this
> + value when bounds-checking transmit writes, and a smaller value
> + underflows that check.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 4
[Severity: High]
This isn't a bug introduced by this patch, but since the binding enforces a
minimum of 4, what happens if a malformed device tree provides a smaller
value?
In drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_parse_dt():
ret = of_property_read_u32(node, "xlnx,tx-fifo-depth",
&fifo->tx_fifo_depth);
if (ret)
return ret;
The value is parsed without enforcing the boundary constraint. Could this
lead to an integer underflow later in axis_fifo_write()?
if (!words_to_write || (len % sizeof(u32)) ||
(words_to_write > (fifo->tx_fifo_depth - 4)))
return -EINVAL;
If fifo->tx_fifo_depth is less than 4, this subtraction underflows, bypassing
the maximum packet size check.
[ ... ]
> + xlnx,use-rx-data:
> + description:
> + Indicates whether the receive data path is present, as configured
> + in the IP core. A value of 1 enables the receive path, 0 disables it.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 0, 1 ]
[Severity: High]
This is a pre-existing issue, but does the driver handle hardware read errors
safely when processing received data?
In drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_read():
bytes_available = ioread32(fifo->base_addr + XLLF_RLR_OFFSET);
words_available = bytes_available / sizeof(u32);
...
if (bytes_available % sizeof(u32)) {
/* this probably can't happen unless IP
* registers were previously mishandled
*/
dev_err(fifo->dt_device, "received a packet that isn't word-aligned\n");
ret = -EIO;
goto err_flush_rx;
}
If the device is in an error state and ioread32() returns 0xFFFFFFFF,
words_available becomes 0x3FFFFFFF. Would this cause the error recovery loop
to stall the CPU?
err_flush_rx:
while (words_available--)
ioread32(fifo->base_addr + XLLF_RDFD_OFFSET);
Executing over a billion memory-mapped reads sequentially could trigger the
kernel's soft lockup watchdog.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260621094312.53655-1-adi25charis@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v10 4/5] iio: adc: versal-sysmon: add threshold event support
From: Jonathan Cameron @ 2026-06-21 16:22 UTC (permalink / raw)
To: Salih Erim
Cc: andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Andy Shevchenko
In-Reply-To: <20260618101414.3462934-5-salih.erim@amd.com>
On Thu, 18 Jun 2026 11:14:13 +0100
Salih Erim <salih.erim@amd.com> wrote:
> Add threshold event support for temperature and supply voltage
> channels.
>
> Temperature events:
> - Rising threshold with configurable value on the device
> temperature channel (current max across all satellites)
> - Per-channel hysteresis as a millicelsius value
> - Event direction is IIO_EV_DIR_RISING (hysteresis mode)
>
> Supply voltage events:
> - Rising/falling threshold per supply channel
> - Per-channel alarm enable via alarm configuration registers
>
> The hardware supports both window and hysteresis alarm modes for
> temperature. This driver uses hysteresis mode, where the upper
> threshold triggers the alarm and the lower threshold clears it
> (re-arm point). The hardware has a single ISR bit per temperature
> channel with no indication of which threshold was crossed, so
> hysteresis mode is the natural fit. The lower threshold register
> is computed internally as (upper - hysteresis).
>
> Hysteresis is stored in the driver as a millicelsius value,
> initialized from the hardware registers at probe. Writing the
> rising threshold or hysteresis recomputes the lower register.
> ALARM_CONFIG is hard-coded to hysteresis mode during init.
>
> The hardware also provides a separate over-temperature (OT)
> threshold, but it is not exposed through IIO as it serves as a
> hardware safety mechanism for platform shutdown. OT will be
> exposed through the thermal framework in a follow-up series.
>
> The interrupt handler masks active threshold interrupts (which are
> level-sensitive) and schedules a delayed worker to poll for condition
> clear before unmasking. When no hardware IRQ is available, event
> specs are not attached and interrupt init is skipped, since the
> I2C regmap backend cannot be called from atomic context.
>
> When disabling a supply channel alarm, the group interrupt remains
> active if any other channel in the same alarm group still has an
> alarm enabled.
>
> A devm cleanup action masks all interrupts on driver unbind to
> prevent unhandled interrupt storms after the IRQ handler is freed.
>
> Signed-off-by: Salih Erim <salih.erim@amd.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
There is some stuff from Sashiko that looks plausible.
https://sashiko.dev/#/patchset/20260618101414.3462934-1-salih.erim%40amd.com
Whilst the out of range temp thresholds might not be a bug
that causes anything particular bad to happen it would be nice
to report an error to userspace if a write is for something we
can't support.
There are some things that I can't figure out without data sheet
diving so I'll leave those for you to sanity check + some I think
we addressed in earlier discussions.
For a few of the things it raises I talk about them inline.
Note I didn't spot anything else (and probably wouldn't have
spotted these :(
Jonathan
> ---
> Changes in v10:
> - Add Reviewed-by tag from Andy Shevchenko
> - Add limits.h include for U16_MAX, S16_MIN, S16_MAX (Andy)
>
> Changes in v9:
> - Add minmax.h include for clamp() (Andy)
> - Join sysmon_supply_thresh_offset to one line, change address
> parameter to unsigned long for consistency (Andy)
> - Combine mask declaration with initialization in
> sysmon_read_event_config (Andy)
> - Rename ier to mask in sysmon_write_event_config for
> consistency with sysmon_read_event_config (Andy)
> - Remove blank line in sysmon_update_temp_lower between
> semantically coupled lines (Andy)
> - Rename unmask to ier (u32) in sysmon_unmask_temp (Andy)
> - Variable name and type consistency audit across all
> event functions (Andy)
>
> Changes in v8:
> - Use MILLIDEGREE_PER_DEGREE in q8p7 conversion functions (Andy)
> - Use regmap_test_bits() in sysmon_read_alarm_config (Andy)
> - Join sysmon_parse_fw signature onto one line (Andy)
> - Fix devm teardown race: replace devm_delayed_work_autocancel
> with INIT_DELAYED_WORK; fold cancel_delayed_work_sync into
> sysmon_disable_interrupts to prevent the worker from
> re-enabling interrupts after the IRQ handler is freed (Sashiko)
> - Drop devm-helpers.h include (no longer needed)
>
> Changes in v7:
> - Move TEMP threshold event onto channel 0; drop OT as
> separate IIO channel -- OT is a hardware safety mechanism
> better suited for the thermal framework follow-up (Jonathan)
> - Use single temp_channels array; attach event spec to
> channel 0 at runtime when IRQ is available, matching the
> pattern used for supply channels (Jonathan)
> - Remove sysmon_temp_thresh_offset; use SYSMON_TEMP_TH_UP
> and SYSMON_TEMP_TH_LOW defines directly at call sites
> - Return administrative state from temp_mask in
> read_event_config instead of transient hardware IMR
> (Jonathan, Sashiko)
> - Add devm_add_action_or_reset to mask all HW interrupts
> on driver unbind (Sashiko)
> - Remove SYSMON_CHAN_TEMP_EVENT macro, SYSMON_ADDR_TEMP_EVENT,
> SYSMON_ADDR_OT_EVENT, SYSMON_BIT_OT, SYSMON_OT_HYST_MASK,
> OT_TH_LOW/UP registers, ot_hysteresis from struct
> - Simplify sysmon_get_event_mask, sysmon_update_temp_lower,
> sysmon_init_hysteresis -- all now operate on single TEMP
> channel only
>
>
> Changes in v6:
> - Remove types.h from header (not needed at any stage) (Andy)
> - Macro brace on separate line for SYSMON_CHAN_TEMP_EVENT (Andy)
> - switch(chan->type) in all event functions instead of cascading
> if statements (Andy)
> - switch(info) in read/write_event_value for nested
> dispatch (Andy)
> - Reversed xmas tree in sysmon_update_temp_lower and
> sysmon_init_hysteresis (Andy)
> - scoped_guard(spinlock_irq) with error check in
> sysmon_unmask_worker (Andy)
> - Combined regmap_read error check with || in
> sysmon_iio_irq (Andy)
> - Join devm_request_irq on one line (Andy)
> - Fix fwnode_irq_get() to propagate only -EPROBE_DEFER;
> treating all negatives as fatal broke probe on I2C nodes
> without interrupts property
>
> Changes in v5:
> - clamp() instead of clamp_t() (Andy)
> - regmap_assign_bits() instead of separate set/clear (Andy)
> - Remove unneeded parentheses (2 places) (Andy)
> - for_each_set_bit on single line (Andy)
> - regmap_clear_bits() instead of regmap_update_bits() (Andy)
> - Simplify unmask XOR to ~status & masked_temp (Andy)
> - Add comment explaining unmask &= ~temp_mask logic (Andy)
> - Split container_of across two lines (Andy)
> - Move ISR write after !isr check to avoid writing 0 (Andy)
> - unsigned int for init_hysteresis address param (Andy)
> - Add comment explaining error check policy in worker/IRQ (Andy)
> - Nested size_add() for overflow-safe allocation (Andy)
> - Propagate negative from fwnode_irq_get() for
> EPROBE_DEFER (Andy)
> - Pass irq instead of has_irq to sysmon_parse_fw (Andy)
>
> Changes in v4:
> - Merge event channels into static temp array; two arrays
> (with/without events) selected by has_irq (Jonathan)
> - Event-only channels have no info_mask; their addresses are
> logical identifiers, not readable registers
> - Drop RAW for voltage events, keep PROCESSED only (Jonathan)
> - Drop scan_type from event channel macro (Jonathan)
> - Blank lines between call+error-check blocks (Jonathan)
> - Fit under 80 chars on one line where possible (Jonathan)
> - default case returns -EINVAL instead of break (Jonathan)
> - sysmon_handle_event: return early in each case (Jonathan)
> - guard(spinlock) in sysmon_iio_irq, return IRQ_NONE/IRQ_HANDLED
> directly (Jonathan)
> - Take irq_lock in write_event_config for temp_mask updates to
> synchronize with unmask worker (Sashiko)
>
> Changes in v3:
> - IWYU: add new includes, group iio headers with blank line (Andy)
> - Reduce casts in millicelsius_to_q8p7, consistent style with
> q8p7_to_millicelsius (Andy)
> - Use clamp_t with typed constants, remove tmp & U16_MAX (Andy)
> - Use !! to return 0/1 from read_alarm_config (Andy)
> - Use regmap_set_bits/clear_bits in write_alarm_config (Andy)
> - Add comment explaining spinlock is safe (I2C never reaches
> event code path) (Andy)
> - Add comment explaining IMR negation logic (Andy)
> - Split read_event_value/write_event_value parameters logically
> across lines (Andy)
> - Move mask/shift after regmap_read error check (Andy)
> - Remove redundant else in read_event_value and
> write_event_value (Andy)
> - Use named constant for hysteresis bit, if-else not ternary
> (Andy)
> - Loop variable declared in for() scope (Andy)
> - Add error checks in sysmon_handle_event (Andy)
> - Use IRQ_RETVAL() macro (Andy)
> - Use devm_delayed_work_autocancel instead of manual INIT +
> devm_add_action (Andy)
> - Use FIELD_GET/FIELD_PREP for hysteresis register bits
> (Jonathan)
> - Split OT vs TEMP handling with FIELD_GET (Jonathan)
> - Rework hysteresis: store as millicelsius value, hardcode
> ALARM_CONFIG to hysteresis mode, compute lower threshold
> from (upper - hysteresis), initialize from HW at probe
> (Jonathan)
> - Remove falling threshold for temperature; single event
> spec per channel with IIO_EV_DIR_RISING (Jonathan)
> - Push IIO_EV_DIR_RISING events for temperature,
> IIO_EV_DIR_EITHER for voltage (Jonathan)
>
> Changes in v2:
> - Reverse Christmas Tree variable ordering in all functions
> - Named constants for hysteresis bits: SYSMON_OT_HYST_BIT,
> SYSMON_TEMP_HYST_BIT instead of magic 0x1/0x2
> - SYSMON_ALARM_BITS_PER_REG replaces magic number 32
> - SYSMON_ALARM_OFFSET() helper macro deduplicates alarm register
> offset computation
> - BIT() macro for shift expressions in conversion functions
> - Hysteresis input validated to single-bit range (0 or 1)
> - Event channels only created when irq > 0 (I2C safety)
> - Group alarm interrupt stays active while any channel in the
> group has an alarm enabled
> - write_event_value returns -EINVAL for unhandled types
> - IRQ_NONE returned for spurious interrupts
> - Q8.7 write path uses multiplication instead of left-shift
> to avoid undefined behavior with negative temperatures
> - (u16) mask prevents garbage in reserved register bits
> - regmap_write return values checked for IER/IDR writes
> - devm cleanup ordering: cancel_work before request_irq
> drivers/iio/adc/versal-sysmon-core.c | 600 ++++++++++++++++++++++++++-
> drivers/iio/adc/versal-sysmon.h | 36 ++
> 2 files changed, 632 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index 03a745d3fb4..50b5228aa22 100644
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
> /*
> * Static temperature channels (always present).
> *
> @@ -52,6 +102,16 @@ static const struct iio_chan_spec temp_channels[] = {
> SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
> };
>
> +static void sysmon_q8p7_to_millicelsius(s16 raw_data, int *val)
> +{
> + *val = (raw_data * MILLIDEGREE_PER_DEGREE) >> SYSMON_FRACTIONAL_SHIFT;
> +}
> +
> +static void sysmon_millicelsius_to_q8p7(u32 *raw_data, int val)
> +{
> + *raw_data = (val << SYSMON_FRACTIONAL_SHIFT) / MILLIDEGREE_PER_DEGREE;
> +}
> +
> static void sysmon_supply_rawtoprocessed(int raw_data, int *val)
> {
> int mantissa, format, exponent;
> @@ -69,6 +129,33 @@ static void sysmon_supply_rawtoprocessed(int raw_data, int *val)
> *val = (mantissa * (int)MILLI) >> exponent;
> }
>
> +static void sysmon_supply_processedtoraw(int val, u32 reg_val, u32 *raw_data)
> +{
> + int exponent = FIELD_GET(SYSMON_MODE_MASK, reg_val);
> + int format = FIELD_GET(SYSMON_FMT_MASK, reg_val);
> + int scale, tmp;
> +
> + scale = BIT(SYSMON_SUPPLY_MANTISSA_BITS - exponent);
> + tmp = (val * scale) / (int)MILLI;
See below. Overflow issue is if val is large enough that this overflows
before tmp is clamped, possibly giving unexpected values.
> +
> + if (format)
> + tmp = clamp(tmp, S16_MIN, S16_MAX);
> + else
> + tmp = clamp(tmp, 0, U16_MAX);
> +
> + *raw_data = (u16)tmp;
> +}
...
> +
> +static int sysmon_read_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int *val, int *val2)
> +{
> + struct sysmon *sysmon = iio_priv(indio_dev);
> + unsigned int reg_val;
> + int offset;
> + int ret;
> +
> + guard(mutex)(&sysmon->lock);
> +
> + switch (chan->type) {
> + case IIO_TEMP:
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + ret = regmap_read(sysmon->regmap, SYSMON_TEMP_TH_UP, ®_val);
> + if (ret)
> + return ret;
> +
> + sysmon_q8p7_to_millicelsius(reg_val, val);
> +
> + return IIO_VAL_INT;
> +
> + case IIO_EV_INFO_HYSTERESIS:
> + *val = sysmon->temp_hysteresis;
> + return IIO_VAL_INT;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + case IIO_VOLTAGE:
> + offset = sysmon_supply_thresh_offset(chan->address, dir);
> + if (offset < 0)
> + return offset;
> +
> + ret = regmap_read(sysmon->regmap, offset, ®_val);
> + if (ret)
> + return ret;
> +
> + sysmon_supply_rawtoprocessed(reg_val, val);
> +
> + return IIO_VAL_INT;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int sysmon_write_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
> +{
> + struct sysmon *sysmon = iio_priv(indio_dev);
> + unsigned int reg_val;
> + u32 raw_val;
> + int offset;
> + int ret;
> +
> + guard(mutex)(&sysmon->lock);
> +
> + switch (chan->type) {
> + case IIO_TEMP:
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + sysmon_millicelsius_to_q8p7(&raw_val, val);
In this path, sashiko is asking whether it is possible for val to be sufficiently large
or negative that the calculation is going to given rather unexpected results.
Given in the read direction you assume it is suitable for passing in a U16, should we
have a check here? + error out if it is out of range?
> +
> + ret = regmap_write(sysmon->regmap, SYSMON_TEMP_TH_UP, raw_val);
> + if (ret)
> + return ret;
> +
> + /* Recompute lower = upper - hysteresis */
> + return sysmon_update_temp_lower(sysmon);
> +
> + case IIO_EV_INFO_HYSTERESIS:
> + if (val < 0)
> + return -EINVAL;
> +
> + sysmon->temp_hysteresis = val;
> +
> + return sysmon_update_temp_lower(sysmon);
> +
> + default:
> + return -EINVAL;
> + }
> +
> + case IIO_VOLTAGE:
> + offset = sysmon_supply_thresh_offset(chan->address, dir);
> + if (offset < 0)
> + return offset;
> +
> + ret = regmap_read(sysmon->regmap, offset, ®_val);
> + if (ret)
> + return ret;
> +
> + sysmon_supply_processedtoraw(val, reg_val, &raw_val);
There is another sashiko report about potential out of range val
here. Probably also want to check for that just as a way to improve
useability.
> +
> + return regmap_write(sysmon->regmap, offset, raw_val);
There is also a comment on whether this is wiping out the fields in the
upper bits of the register. If it isn't (maybe they are read only?)
then a comment here would be good.
> +
> + default:
> + return -EINVAL;
> + }
> +}
^ permalink raw reply
* Re: [PATCH v10 5/5] iio: adc: versal-sysmon: add oversampling support
From: Jonathan Cameron @ 2026-06-21 16:28 UTC (permalink / raw)
To: Salih Erim
Cc: andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Andy Shevchenko
In-Reply-To: <20260618101414.3462934-6-salih.erim@amd.com>
On Thu, 18 Jun 2026 11:14:14 +0100
Salih Erim <salih.erim@amd.com> wrote:
> Add support for reading and writing the oversampling ratio through
> the IIO oversampling_ratio attribute. The hardware supports averaging
> 2, 4, 8, or 16 samples, plus a ratio of 1 (no averaging).
>
> Temperature and supply channels share oversampling configuration at
> the type level (all temperature channels share one ratio, all supply
> channels share another), exposed through info_mask_shared_by_type.
>
> The hardware encoding uses sample_count / 2 in a 4-bit field within
> the CONFIG register. Per-channel averaging enable registers must also
> be updated to activate or deactivate averaging.
>
> Signed-off-by: Salih Erim <salih.erim@amd.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Sashiko I think correctly calls out that you should have
the oversampling related bits set in info masks for the statically
declared temp channels as well.
Otherwise looks good to me.
I will note that sashiko was busy complaining about at least
some of these in earlier versions. Please always take a look
before sending next version. If it is spouting garbage (which
it does for I2C dma safety for instance) then best option I think
is to reply to your own posting saying what is wrong and what is
correct and to be addressed.
Thanks,
Jonathan
> ---
> Changes in v10:
> - No code changes
>
> Changes in v9:
> - Add Reviewed-by tag from Andy Shevchenko
> - No code changes
>
> Changes in v8:
> - Use unsigned int for val parameter and hw_val in both
> osr_write helpers (Andy)
> - Use ~0 instead of ~0U for avg enable bitmask (Andy)
>
> Changes in v7:
> - Split sysmon_osr_write into sysmon_osr_write_temp and
> sysmon_osr_write_supply; caller dispatches with if/else
> on chan->type (Jonathan)
> - Restore HW encoding comment in both helpers; fix
> cross-reference in sysmon_osr_write_supply
>
> Changes in v6:
> - Fix FIELD_PREP indentation in sysmon_osr_write (Andy)
> - unsigned int for loop index in sysmon_write_raw (Andy)
>
> Changes in v5:
> - Remove unneeded parentheses in i * SYSMON_REG_STRIDE (Andy)
> - Use struct regmap *map local variable in
> sysmon_set_avg_enable (Andy)
> - switch instead of redundant if/if on channel_type (Andy)
> - Add CONFIG register readback fence after oversampling update
> to prevent NoC bus hang from posted writes (found during
> hardware stress testing)
>
> Changes in v4:
> - Return directly from sysmon_set_avg_enable calls, remove
> else after early returns, drop unreachable return 0 (Jonathan)
> - Rename mask defines to SYSMON_CONFIG_SUPPLY_OSR and
> SYSMON_CONFIG_TEMP_SAT_OSR (Jonathan)
> - Drop "bits X:Y" from GENMASK comments (Jonathan)
> - Blank lines after if (ret) return ret blocks (Jonathan)
> - Move oversampling read inside guard(mutex) scope
>
> Changes in v3:
> - No changes
>
> Changes in v2:
> - EN_AVG per-channel bitmask registers written with all-ones
> instead of boolean 1 when oversampling is enabled
> - EN_AVG write errors propagated to userspace
> - Oversampling limited to satellite temp and supply channels;
> static temp channels do not participate
> - Oversampling exposes actual sample counts (1,2,4,8,16) to
> userspace with internal HW register translation
> - write_raw_get_fmt returns IIO_VAL_INT for oversampling ratio
> - HW encoding documented (sample_count/2, not log2)
> - oversampling_avail is const int[] (type match fix)
> drivers/iio/adc/versal-sysmon-core.c | 153 ++++++++++++++++++++++++++-
> drivers/iio/adc/versal-sysmon.h | 17 +++
> 2 files changed, 169 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index 50b5228aa22..bae229f27c6 100644
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
> +
> static int sysmon_read_label(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> char *label)
> @@ -464,6 +602,9 @@ static int sysmon_read_label(struct iio_dev *indio_dev,
>
> static const struct iio_info sysmon_iio_info = {
> .read_raw = sysmon_read_raw,
> + .write_raw = sysmon_write_raw,
> + .write_raw_get_fmt = sysmon_write_raw_get_fmt,
> + .read_avail = sysmon_read_avail,
> .read_label = sysmon_read_label,
> .read_event_config = sysmon_read_event_config,
> .write_event_config = sysmon_write_event_config,
> @@ -755,6 +896,10 @@ static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev, int ir
> .indexed = 1,
> .address = reg,
> .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .info_mask_shared_by_type =
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .info_mask_shared_by_type_available =
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> .event_spec = irq > 0 ?
> sysmon_supply_events : NULL,
> .num_event_specs = irq > 0 ?
> @@ -786,7 +931,11 @@ static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev, int ir
> .address = SYSMON_TEMP_SAT_BASE +
> (reg - 1) * SYSMON_REG_STRIDE,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_type =
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .info_mask_shared_by_type_available =
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> .datasheet_name = label,
This stuff also needs to be set for the static channels as it applies
to them as well. Obviously it only makes a practical difference if
you have no satellite sensors (in which case it makes little sense anyway)
but it is the 'right' thing to do to document they apply there as well.
> };
> }
> @@ -833,6 +982,8 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
>
> sysmon = iio_priv(indio_dev);
> sysmon->regmap = regmap;
> + sysmon->temp_oversampling = 1;
> + sysmon->supply_oversampling = 1;
>
^ permalink raw reply
* Re: [PATCH v2 4/4] iio: dac: ad3530r: Add support for AD3532R/AD3532
From: Jonathan Cameron @ 2026-06-21 16:45 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Kim Seer Paller, David Lechner, Nuno Sá, Andy Shevchenko,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, linux-kernel, linux, devicetree
In-Reply-To: <ai_OeEegWavHcNF1@ashevche-desk.local>
On Mon, 15 Jun 2026 13:05:44 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Mon, Jun 15, 2026 at 02:20:18PM +0800, Kim Seer Paller wrote:
> > The AD3532R/AD3532 is a 16-channel, 16-bit voltage output DAC with a
> > dual-bank register architecture (bank 0 at 0x1000 for channels 0-7,
> > bank 1 at 0x3000 for channels 8-15). It shares similar functionality
> > with AD3530R (channel configuration, LDAC triggering, powerdown control),
> > the main difference being the register address map due to the dual-bank
> > architecture, handled by table-driven helpers.
> >
> > Add AD3532R-specific register definitions, channel specs, per-bank
> > register arrays, a dedicated ad3532r_set_dac_powerdown(), and per-chip
> > regmap_config to limit debugfs-exposed register space to each variant's
> > actual address range.
>
> ...
>
>
> > help
> > - Say yes here to build support for Analog Devices AD3530R, AD3531R
> > - Digital to Analog Converter.
> > + Say yes here to build support for Analog Devices AD3530/AD3530R,
> > + AD3531/AD3531R, and AD3532/AD3532R Digital to Analog Converters.
>
> This just shows how unscalable the above text is. That's why we usually
> recommend to make the list explicit and separated.
>
> Say yes here to build support for the following Analog Devices
> Digital to Analog Converters:
> - AD3530/AD3530R (8-channel)
> - AD3531/AD3531R (4-channel)
> - AD3532/AD3532R (16-channel)
>
> (and looking into the C-file change, perhaps add here as well distinctive
> information, such as number of channels, in the parentheses).
>
> > To compile this driver as a module, choose M here: the
> > module will be called ad3530r.
>
> ...
>
> > +#define AD3532R_INTERFACE_CONFIG_A_0 0x1000
> > +#define AD3532R_INTERFACE_CONFIG_A_1 0x3000
> > +#define AD3532R_OUTPUT_OPERATING_MODE_0 0x1020
> > +#define AD3532R_OUTPUT_OPERATING_MODE_1 0x1021
> > +#define AD3532R_OUTPUT_OPERATING_MODE_2 0x3020
> > +#define AD3532R_OUTPUT_OPERATING_MODE_3 0x3021
> > +#define AD3532R_OUTPUT_CONTROL_0 0x102A
> > +#define AD3532R_OUTPUT_CONTROL_1 0x302A
> > +#define AD3532R_REFERENCE_CONTROL_0 0x103C
> > +#define AD3532R_REFERENCE_CONTROL_1 0x303C
> > +#define AD3532R_SW_LDAC_TRIG_0 0x10E5
> > +#define AD3532R_SW_LDAC_TRIG_1 0x30E5
> > +#define AD3532R_INPUT_CH_0 0x10EB
> > +#define AD3532R_INPUT_CH_1 0x30EB
> > +#define AD3532R_MAX_REG_ADDR 0x30F9
Whilst we are here, Sashiko thinks there is an off by one on that value
as it's the lower of the two registers that make up channel 15.
https://sashiko.dev/#/patchset/20260615-iio-ad3532r-support-v2-0-84a0af8b83fa%40analog.com
It also suggests an existing bug that it would be good to look into.
>
> Hmm... I dunno if it's better to sort by values (so the "bank" 0 goes together
> followed by "bank" 1). Jonathan, what's your preference here? Nuno, David?
That is how people will typically check them vs the datasheet so I
agree with numeric order. Maybe with a comment at the top about there
effectively being two banks. Many of the registers are effectively copies
for the new channels but not all of them, so a macro approach would
probably be even more confusing.
Jonathan
^ permalink raw reply
* Re: [PATCH v2 1/4] iio: dac: ad3530r: Refactor setup to table-driven register bank approach
From: Jonathan Cameron @ 2026-06-21 16:49 UTC (permalink / raw)
To: Kim Seer Paller
Cc: David Lechner, Nuno Sá, Andy Shevchenko, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
linux-kernel, linux, devicetree
In-Reply-To: <20260615-iio-ad3532r-support-v2-1-84a0af8b83fa@analog.com>
On Mon, 15 Jun 2026 14:20:15 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:
> Replace direct register calls in ad3530r_setup() with per-chip register
> address arrays and bank helpers (ad3530r_set_reg_bank_bits,
> ad3530r_write_reg_banks). Convert sw_ldac_trig_reg from a static
> register address to a function pointer for per-bank LDAC trigger
> register selection. Switch spi_device_id to named initializers.
>
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
>
> static const struct spi_device_id ad3530r_id[] = {
> - { "ad3530", (kernel_ulong_t)&ad3530_chip },
> - { "ad3530r", (kernel_ulong_t)&ad3530r_chip },
> - { "ad3531", (kernel_ulong_t)&ad3531_chip },
> - { "ad3531r", (kernel_ulong_t)&ad3531r_chip },
> + { .name = "ad3530", .driver_data = (kernel_ulong_t)&ad3530_chip },
> + { .name = "ad3530r", .driver_data = (kernel_ulong_t)&ad3530r_chip },
> + { .name = "ad3531", .driver_data = (kernel_ulong_t)&ad3531_chip },
> + { .name = "ad3531r", .driver_data = (kernel_ulong_t)&ad3531r_chip },
There is a high chance this will cross with Uwe's series that does
this change for all SPI drivers. Before you send each version have
a quick look to see if I've already picked that up. Or perhaps
better to just pick that up your self as a dependency to make
life easy. Just say you have done that in the cover letter.
> { }
> };
> MODULE_DEVICE_TABLE(spi, ad3530r_id);
>
^ permalink raw reply
* Re: [PATCH v2 2/4] irqchip/gic-v3: Refactor GIC600 limited to 32bit PA erratum handling
From: Marc Zyngier @ 2026-06-21 16:59 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-pci, Krzysztof Wilczyński, Bjorn Helgaas,
Catalin Marinas, Conor Dooley, Geert Uytterhoeven,
Krzysztof Kozlowski, Lorenzo Pieralisi, Manivannan Sadhasivam,
Rob Herring, Yoshihiro Shimoda, devicetree, linux-arm-kernel,
linux-doc, linux-kernel, linux-renesas-soc
In-Reply-To: <20260618220427.14325-3-marek.vasut+renesas@mailbox.org>
On Thu, 18 Jun 2026 23:02:00 +0100,
Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
>
> The GIC600 implementation is now known to be used on multiple 64-bit
> SoCs, where it has address width for AXI or APB interface configured
> to 32 bit, and it can access only the first 4GiB of physical address
> space.
>
> Rework the handling of the quirk to work around this limitation such
> that new entries can be added purely as new compatible strings, with
> no need to add additional functions or new its_quirk array entries.
>
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Acked-by: Marc Zyngier <maz@kernel.org>
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH v2 3/4] irqchip/gic-v3: Add Renesas R-Car Gen4 erratum workaround
From: Marc Zyngier @ 2026-06-21 17:00 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-pci, Yoshihiro Shimoda, Krzysztof Wilczyński,
Bjorn Helgaas, Catalin Marinas, Conor Dooley, Geert Uytterhoeven,
Krzysztof Kozlowski, Lorenzo Pieralisi, Manivannan Sadhasivam,
Rob Herring, devicetree, linux-arm-kernel, linux-doc,
linux-kernel, linux-renesas-soc
In-Reply-To: <20260618220427.14325-4-marek.vasut+renesas@mailbox.org>
On Thu, 18 Jun 2026 23:02:01 +0100,
Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
>
> Renesas R-Car S4/V4H/V4M GIC600 integration has address width for AXI
> or APB interface configured to 32 bit, it can therefore access only
> the first 4 GiB of physical address space. This information comes from
> R-Car V4H Interface Specification sheet, there is currently no technical
> update number assigned to this limitation. Further input from hardware
> engineer indicates that this limitation also applies to R-Car S4 and V4M.
> Name the limitation GEN4GICITS1, and add a driver quirk to mitigate this
> limitation.
>
> The quirk is keyed on the combination of the GIC implementation
> and the platform identification in the device tree.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
With the SoB chain issue addressed:
Acked-by: Marc Zyngier <maz@kernel.org>
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: David Heidelberg @ 2026-06-21 17:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang,
Bjorn Andersson, Konrad Dybcio, linux-input, devicetree,
linux-kernel, linux-arm-msm, phone-devel, Krzysztof Kozlowski,
Konrad Dybcio
In-Reply-To: <1d0e7e31-f808-4347-955a-7246dea208f5@ixit.cz>
On 28/05/2026 00:13, David Heidelberg wrote:
> On 27/05/2026 23:56, Dmitry Torokhov wrote:
>> Hi David,
>>
>> On Sat, May 23, 2026 at 11:45:35AM +0200, David Heidelberg via B4 Relay wrote:
>>> From: David Heidelberg <david@ixit.cz>
>>>
>>> We know the driver is reporting s3706b, introduce the compatible so we
>>> can more easily introduce quirks for weird touchscreen replacements in
>>> followup series.
>>>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>> ---
>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/
>>> arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> index 6b7378cf4d493..148164d456a5a 100644
>>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> @@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
>>> };
>>> };
>>> &i2c12 {
>>> status = "okay";
>>> clock-frequency = <400000>;
>>> synaptics-rmi4-i2c@20 {
>>> - compatible = "syna,rmi4-i2c";
>>> + compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
>>
>> So I believe we established that this device (s3706b) does not in fact
>> implement rmi4 protocol properly. Why do we have "syna,rmi4-i2c" as a
>> fallback? Shouldn't it be just "syna,rmi4-s3706b"?
>
> The vendor supplies s3706b which does implement the RMI4 properly.
>
> The 3rd party replacement impersonating original parts may not implement it
> properly, but I don't address this issue in this initial submission.
>
> With this compatible we know which original part is used by the vendor and
> installed in the phones, so later we can deduct specific sequences for the
> replacement aftermarket parts to keep phone touchscreen working same as they do
> on Android without affecting other devices.
Hello Dmitry.
May I ask what is currently preventing this series from moving forward?
The first version was posted in 2023 [1]. I picked it up again in 2025 [2] and
am now on the 9th iteration (this patchset). At this point, the series has been
under discussion for well over a year, with relatively little feedback and
increasingly long gaps between review rounds.
The current approach is based on the guidance I have received so far, including
suggestions from the device-tree maintainers. When concerns were raised, I tried
to address them and rework the series accordingly.
What I am struggling with is understanding what specific issue still needs to be
resolved before these patches can be accepted. If there are remaining
requirements, objections to the approach, or technical concerns that I have not
addressed, I would appreciate having them stated explicitly so I can work on them.
I also split out the straightforward, self-contained changes in the hope that at
least those could progress independently while I continued working on any
follow-up requirements. However, even those patches do not appear to be moving
forward.
Could you please clarify what outcome you would like to see from this series,
and what concrete changes would be required to get it accepted?
Thank you,
David
[1]
https://lore.kernel.org/all/20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org/
[2] https://lore.kernel.org/all/20250402-synaptics-rmi4-v4-0-1bb95959e564@ixit.cz/
>
> David
>
>>
>> Thanks.
>>
>
--
David Heidelberg
^ 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