Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] dt-bindings: iio: st,st-sensors: add st,fullscale-mg
From: Jonathan Cameron @ 2026-06-21 12:41 UTC (permalink / raw)
  To: me
  Cc: github.com, linusw, denis.ciocca, robh, krzk+dt, conor+dt,
	dlechner, nuno.sa, andy, sanjayembeddedse, maudspierings,
	linux-iio, devicetree, linux-kernel
In-Reply-To: <6475e269775789d61ade326190b0d234@herrie.org>

On Mon, 15 Jun 2026 13:16:52 +0200
me@herrie.org wrote:

> On 2026-06-14 20:44, Jonathan Cameron wrote:
> > On Fri,  5 Jun 2026 12:08:42 +0200
> > Herman van Hazendonk <github.com@herrie.org> wrote:
> >   
> >> Add an optional st,fullscale-mg property that selects the initial
> >> full-scale range of an ST MEMS sensor at probe time, expressed in
> >> milligauss for magnetometers (and analogous engineering units for
> >> other ST sensor families that may grow this property in the future).
> >> 
> >> The property is purely additive: if absent, drivers fall back to
> >> their existing chip default, and if present but unsupported by the
> >> specific sensor the driver warns and falls back. No existing in-tree
> >> DTS is affected.
> >> 
> >> The motivating case is the LSM303DLH magnetometer on the HP TouchPad
> >> (apq8060 / tenderloin) where the kernel's chip-default +/-1.3 G range
> >> saturates the X axis to the chip's 0xF000 overflow sentinel out of
> >> probe, because the chip is mounted close to surrounding power planes
> >> and picks up enough DC bias to exceed the smallest range. The driver
> >> core hardcodes fs_avl[0] as the starting range, so userspace cannot
> >> recover without racing the driver to write the in_magn_x_scale sysfs
> >> attribute after probe. st,fullscale-mg lets the device tree declare
> >> a wider initial range up-front and avoids the race entirely.  
> > 
> > I'm trying to understand what you mean here by racing.
> > 
> > If we get this overflow condition the chip is wedged until reset, or
> > userspace simply has to change the range to recover?
> > 
> > I'm wondering if a UDEV rule is sufficient in theory to fix this.
> > 
> > I'm not necessarily against having the range in DT as it is effectively
> > hardware dependent but just want to make sure I fully understand the 
> > issue.
> > 
> > Jonathan
> > 
> > 
> >   
> >> 
> >> Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
> >> ---
> >>  .../devicetree/bindings/iio/st,st-sensors.yaml | 18 
> >> ++++++++++++++++++
> >>  1 file changed, 18 insertions(+)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/iio/st,st-sensors.yaml 
> >> b/Documentation/devicetree/bindings/iio/st,st-sensors.yaml
> >> index a1a958215cdb..335f38e9f78f 100644
> >> --- a/Documentation/devicetree/bindings/iio/st,st-sensors.yaml
> >> +++ b/Documentation/devicetree/bindings/iio/st,st-sensors.yaml
> >> @@ -126,6 +126,24 @@ properties:
> >>    mount-matrix:
> >>      description: an optional 3x3 mounting rotation matrix.
> >> 
> >> +  st,fullscale-mg:
> >> +    description: |
> >> +      Selects the initial sensor full-scale at probe time, expressed 
> >> in
> >> +      milligauss for magnetometers (or analogous engineering units 
> >> for
> >> +      other sensor families that may grow this property in the 
> >> future).
> >> +      The value must match one of the sensor-specific full-scale 
> >> ranges
> >> +      supported by the chip; if the chip does not support the 
> >> requested
> >> +      range the driver falls back to its built-in default.
> >> +
> >> +      This is intended for boards where the magnetometer chip picks 
> >> up
> >> +      enough DC bias from nearby PCB structures (power planes, 
> >> ferrous
> >> +      shields, etc.) that the kernel's chip-default 
> >> highest-sensitivity
> >> +      range saturates one or more axes to the chip's overflow 
> >> sentinel,
> >> +      and userspace observes that axis as permanently stuck. 
> >> Declaring
> >> +      a wider initial range avoids the saturation at the cost of a
> >> +      slightly coarser quantisation.
> >> +    $ref: /schemas/types.yaml#/definitions/uint32
> >> +
> >>  allOf:
> >>    - if:
> >>        properties:  
> Hi Jonathan,
> 
> "racing" was loose wording on my part. The chip is not wedged. Once 
> userspace writes a wider range to in_magn_x_scale,
> the next conversion comes back with sensible data and everything works. 
> A UDEV rule on add of the IIO device would, in principle, fix
> the steady-state problem.
> 
> What I was clumsily pointing at is the probe-time window: the IIO 
> consumers in our stack (sensorfw's iio-sensors-adaptor, geomagnetic
> / orientation services) start polling in_magn_x_raw essentially the 
> moment the device node appears, and they treat the saturated
> 0xF000 sentinel as a legitimate sample rather than as overflow. 

Should this perhaps be returning -ERANGE or similar to indicate a failed
read via the return value rather than reading out something
that requires device specific userspace to handle?

Not that this changes the requirements as obviously it still doesn't work
when we want a reading!

> Until 
> the UDEV rule fires and the kernel commits the new range, every
> read returns the stuck sentinel, so orientation/compass features are 
> wrong from boot for some non-trivial number of samples (and on
> slow-boot paths the consumer may have already cached a bogus calibration 
> baseline by the time UDEV catches up).
> 
> So the trade-off as I see it is:
> 
> - UDEV rule: works for steady state, fully out-of-tree, but the 
> per-board configuration ends up split between two places (DTS for
> "this board has an LSM303DLH at i2c@... in this orientation"; UDEV for 
> "and by the way it needs a wider range or its raw readings are
> nonsense"). The wider-range requirement is purely a property of how the 
> magnetometer is mounted on the board, which is what DT is for.

Fair enough to it being board dependent.

> 
> - st,fullscale-mg in DT: keeps all hardware-dependent calibration in one 
> place, available before any IIO consumer can open the device,
> and harmless on boards that don't need it (absent property existing 
> chip-default behaviour, no   DTS in tree changes).
> 
> I'm happy to drop the wording about "racing" in v2 and replace it with a 
> more accurate description of the early-consumer issue if that
> reads better. And of course if you'd rather we ship a UDEV rule 
> downstream instead of adding a binding, I'll yield, but my read is that
> this is hardware-dependent enough to belong in DT.
I'm fine with the DT solution with some of this extra info captured
in the commit message.

Thanks,

Jonathan

> 
> Thanks,
> Herman


^ permalink raw reply

* Re: [PATCH 2/5] riscv: dts: spacemit: enable the i2c2 adapter on K3 Pico-ITX board
From: Aurelien Jarno @ 2026-06-21 13:14 UTC (permalink / raw)
  To: linux-kernel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Yixun Lan
  Cc: Aurelien Jarno,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:RISC-V ARCHITECTURE,
	open list:RISC-V SPACEMIT SoC Support
In-Reply-To: <20260620062948.0A1161F000E9@smtp.kernel.org>

Both issues reported by Sashiko AI (see below) are valid, I'll submit a 
v2 fixing them, but I'll first leave a few more days for humans to also 
have a look at the patches.

Regards
Aurelien

On 2026-06-20 06:29, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] The pinctrl subnode is named `i2c2-0-pins` but belongs to the `i2c2-1-cfg` parent node.
> - [Low] Missing space before the assignment operator in `bias-pull-up= <0>;`.
> --
> 
> commit 4dc6c92d42831e85b38f3350c15707d0e2b3af18
> Author: Aurelien Jarno <aurelien@aurel32.net>
> 
> riscv: dts: spacemit: enable the i2c2 adapter on K3 Pico-ITX board
> 
> This commit defines the I2C2 pinctrl properties and enables the adapter
> on the K3 Pico-ITX board. It will be used by the 24c02 eeprom.
> 
> > diff --git a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> > index 23899d3f308a0..c9fe9c2e5aeee 100644
> > --- a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> 
> [ ... ]
> 
> > @@ -45,6 +45,17 @@ gmac0-phy-0-pins {
> >  		};
> >  	};
> >  
> > +	/omit-if-no-ref/
> > +	i2c2_1_cfg: i2c2-1-cfg {
> > +		i2c2-0-pins {
> 
> [Severity: Low]
> This isn't a bug, but should this subnode be named i2c2-1-pins to match
> the i2c2-1-cfg parent node? It seems to diverge from the standard naming
> pattern where the suffix of the parent and child matches.
> 
> > +			pinmux = <K3_PADCONF(46, 5)>,	/* i2c2 scl */
> > +				 <K3_PADCONF(47, 5)>;	/* i2c2 sda */
> > +
> > +			bias-pull-up= <0>;
> 
> [Severity: Low]
> This isn't a bug, but there is a missing space before the equals sign
> here, which diverges from the spacing used in the surrounding code.
> 
> > +			drive-strength = <25>;
> > +		};
> > +	};
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260620062238.3199108-1-aurelien@aurel32.net?part=2
> 

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net

^ permalink raw reply

* Re: [PATCH v4 02/13] dt-bindings: clock: qcom,qcm2290-dispcc: Add DSI1 PHY and sleep clocks
From: Imran Shaik @ 2026-06-21 13:25 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-eager-lynx-of-eternity-f77ecb@quoll>



On 05-06-2026 03:53 pm, Krzysztof Kozlowski wrote:
> On Thu, Jun 04, 2026 at 10:56:08AM +0530, Imran Shaik wrote:
>> Update the QCM2290 DISPCC binding to document additional clock inputs
>> supported by the hardware, including DSI1 PHY byte/pixel clocks and
>> the sleep clock, alongside the existing clock list. This is an ABI
>> extension, and existing clock inputs ordering is unchanged.
> 
> That's ABI break, not extension, because you require all these clocks.
> And "dtbs_check" would tell you that it is a break.
> 
> You need to provide reasons why they have to be added - something was
> not working? Something was missing? Did it matter?
> 

Apologies for the late reply.

The DISPCC hardware supports additional external clocks (DSI1 PHY 
byte/pixel and sleep clocks) which are currently missing from the 
binding. I will capture these details in commit text and update the ABI 
break details in next series.

Thanks,
Imran

^ permalink raw reply

* Re: [PATCH v4 03/13] dt-bindings: clock: qcom: Add Qualcomm Shikra Display clock controller
From: Imran Shaik @ 2026-06-21 13:26 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-bipedal-aquamarine-tuatara-e1fd62@quoll>



On 05-06-2026 03:55 pm, Krzysztof Kozlowski wrote:
> On Thu, Jun 04, 2026 at 10:56:09AM +0530, Imran Shaik wrote:
>> The Qualcomm Shikra Display clock controller is similar to QCM2290
>> DISPCC hardware block. Hence, reuse the QCM2290 DISPCC bindings for
> 
> Similar or compatible? You are not reusing the bindings here, but
> stating that Shikra is compatible with QCM2290. Just say that.
> 

Sure, I will update that the Shikra DISPCC is compatible with QCM2290 
DISPCC in the next series.

Thanks,
Imran

>> Qualcomm Shikra SoC.
>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>>   Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Best regards,
> Krzysztof
> 


^ permalink raw reply

* Re: [PATCH v4 04/13] dt-bindings: clock: qcom: Add Qualcomm Shikra GPU clock controller
From: Imran Shaik @ 2026-06-21 13:27 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-camouflaged-seagull-of-chaos-afeaf5@quoll>



On 05-06-2026 03:54 pm, Krzysztof Kozlowski wrote:
> On Thu, Jun 04, 2026 at 10:56:10AM +0530, Imran Shaik wrote:
>> The Qualcomm Shikra GPU clock controller is similar to QCM2290 GPUCC
>> hardware block, with minor differences. Hence, reuse the QCM2290 GPUCC
> 
> No header file? Are you going to reuse the QCM one, so basically you
> have the same clocks?
> 

Yes, Shikra GPUCC has the same clocks as QCM2290 GPUCC, and re-suing the 
QCM2290 header file. I will update these details in the commit text in 
next series.

Thanks,
Imran

^ permalink raw reply

* Re: [PATCH v4 01/13] clk: qcom: gcc-qcm2290: Keep the critical clocks always-on from probe
From: Imran Shaik @ 2026-06-21 13:28 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: <w2y2wz3cbifnko3td3ovxfom26lll3vl5qgulksa7qnuvzukpf@pqlrh23sljsz>



On 06-06-2026 04:54 pm, Dmitry Baryshkov wrote:
> On Thu, Jun 04, 2026 at 10:56:07AM +0530, Imran Shaik wrote:
>> Some GCC branch clocks are required to be kept always-on due to the
>> hardware requirements. Drop the modelling of those always-on QCM2290 GCC
>> clocks and use the latest .clk_cbcr convention to keep them enabled from
>> probe.
> 
> You got the feedback, but it got ignored. There is no explanation about
> gcc_gpu_iref_clk and several other clocks becoming always on.
> 

Apologies for the late reply.

I have updated generically that few clocks are required to be kept ON 
due to the hardware requirements. For gcc_gpu_iref_clk clock the 
Power-On-Reset value is default ON, and such clocks are kept ON from 
probe. As Agatti is working functionally, I will keep this clock 
modelling same as before.

And I will update commit text that the GCC video ahb/xo clocks are 
required to be kept ON similar to other camera/disp ahb/xo clocks.

Thanks,
Imran

>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>>   drivers/clk/qcom/gcc-qcm2290.c | 160 +++++------------------------------------
>>   1 file changed, 18 insertions(+), 142 deletions(-)
>>
> 


^ permalink raw reply

* 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, &reg_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, &reg_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, &reg_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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox