* [PATCH 1/3] arm64: dts: imx94-xspi: add the DMA channels
From: han.xu @ 2026-06-17 21:55 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Han Xu, devicetree, imx, linux-arm-kernel, linux-kernel
From: Han Xu <han.xu@nxp.com>
Add the DMA channels for iMX94 XSPI controller.
Signed-off-by: Han Xu <han.xu@nxp.com>
---
arch/arm64/boot/dts/freescale/imx94.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx94.dtsi b/arch/arm64/boot/dts/freescale/imx94.dtsi
index c460ece6070f8..9de1b9754450e 100644
--- a/arch/arm64/boot/dts/freescale/imx94.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx94.dtsi
@@ -820,6 +820,8 @@ xspi1: spi@42b90000 {
#size-cells = <0>;
clocks = <&scmi_clk IMX94_CLK_XSPI1>;
clock-names = "per";
+ dmas = <&edma2 27 0 0>, <&edma2 28 0 FSL_EDMA_RX>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -836,6 +838,8 @@ xspi2: spi@42be0000 {
#size-cells = <0>;
clocks = <&scmi_clk IMX94_CLK_XSPI2>;
clock-names = "per";
+ dmas = <&edma4 42 0 0>, <&edma4 43 0 FSL_EDMA_RX>;
+ dma-names = "tx", "rx";
status = "disabled";
};
};
--
2.34.1
^ permalink raw reply related
* Re: [RFC PATCH v3 3/5] iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes
From: David Lechner @ 2026-06-17 21:43 UTC (permalink / raw)
To: Marcelo Schmitt, linux-iio, devicetree, linux-kernel
Cc: jic23, nuno.sa, Michael.Hennerich, andy, robh, krzk+dt, conor+dt,
pop.ioan-daniel, marcelo.schmitt1
In-Reply-To: <a09f2cac2d26fe385f1f7790b0db64b7d9953ac6.1781661028.git.marcelo.schmitt@analog.com>
On 6/16/26 9:03 PM, Marcelo Schmitt wrote:
> Some devices using DMAengine buffers are connected to extra hardware that
> allows setting how fast data is transferred to/from the buffer. However,
> those extra pieces of harwdware are external to the sensor chip such that
> supporting the transfer speed as a sensor property is a bit of an
> inaccuracy. Expand IIO DMAengine buffer interfaces to take arguments for
> extra sysfs attributes, enabling the transfer speed to be configured
> through the buffer interface.
This message is a bit confusing. It sounds like it is attempting to
control something about the DMA controller itself. But based on the
later patches, it looks like this is just so we can add arbitrary
sysfs attributes to the bufferX directory. And in this specific case,
a sampling_frequency attribute.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> New patch.
>
> Now that I've come to this buffer "solution", I have pretty much convinced
> myself it would be better to instead have some sort of IIO trigger to control
> the signal source connected to SPI offloading trigger module.
>
In the other chips with SPI offload we've done already, we just used
IIO_CHAN_INFO_SAMP_FREQ to control the SPI offload trigger rate.
Any reason why we can't do that here? In the original SPI offload
discussions, IIRC the general consensus was that adding a trigger
just to control that was overkill when I suggested the same.
^ permalink raw reply
* Re: [PATCH v4 1/3] regulator: qcom-refgen: correct the regulator type to CURRENT
From: Dmitry Baryshkov @ 2026-06-17 21:42 UTC (permalink / raw)
To: Kathiravan Thirumoorthy
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Konrad Dybcio, linux-arm-msm, linux-kernel,
devicetree, stable, Konrad Dybcio
In-Reply-To: <20260617-ipq9650_refgen-v4-1-c505ea6c6661@oss.qualcomm.com>
On Wed, Jun 17, 2026 at 11:08:43PM +0530, Kathiravan Thirumoorthy wrote:
> As per the REFGEN IP team, this block supplies the reference current to
> the PHYs in the SoC. So, correct the regulator type to REGULATOR_CURRENT
> to match with the HW behavior.
>
> Fixes: 7cbfbe237960 ("regulator: Introduce Qualcomm REFGEN regulator driver")
> Cc: stable@vger.kernel.org
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
> ---
> drivers/regulator/qcom-refgen-regulator.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH RFC v4 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset bindings
From: Conor Dooley @ 2026-06-17 21:41 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260617-deed-snap-4649ffae0e27@spud>
[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]
On Wed, Jun 17, 2026 at 10:23:56PM +0100, Conor Dooley wrote:
> On Wed, Jun 17, 2026 at 08:47:53PM +0300, Stefan Dösinger wrote:
> > Am Mittwoch, 17. Juni 2026, 19:08:02 Ostafrikanische Zeit schrieb Conor
> > Dooley:
> > > On Tue, Jun 16, 2026 at 11:26:21PM +0300, Stefan Dösinger wrote:
> >
> > > > +F: Documentation/devicetree/zte,zx297520v3-*
> > >
> > > Sashiko complaint here looks valid.
> >
> > Yes, it is valid.
> >
> > It also brought up another concern in patch 12 that has an impact on this
> > binding:
> >
> > Am Dienstag, 16. Juni 2026, 23:42:19 Ostafrikanische Zeit schrieb sashiko-
> > bot@kernel.org:
> > > issue(s) to consider: - [Low] The `syscon-reboot` node is incorrectly
> > > defined at the root level using the explicitly deprecated `regmap`
> > > property. --
> >
> > And indeed "regmap" is deprecated and I missed it somehow. As far as I
> > understand, to put syscon-reboot as a child of the clock, I need to add the
> > simple-mfd .compatible:
> >
> > Either
> >
> > topclk: clock-controller@13b000 {
> > compatible = "zte,zx297520v3-topclk", "syscon", "simple-mfd";
> > reg = <0x0013b000 0x400>;
> > ...
> > syscon-reboot {
> > compatible = "syscon-reboot";
> > regmap = <&topclk>;
> > offset = <0x0>;
> > mask = <0x1>;
> > };
> > };
> >
> > --- or ---
> >
> > something@13b000 {
> > compatible = "zte,zx297520v3-topcrm", "syscon", "simple-mfd";
> > reg = <0x0013b000 0x400>;
> > ranges;
> >
> > topclk: clock-controller@0 {
> > compatible = "zte,zx297520v3-topclk";
> > ...
> > }
> >
> > syscon-reboot {
> > compatible = "syscon-reboot";
> > offset = <0x0>;
> > mask = <0x1>;
> > };
> > };
> >
> > I see both ways in existing DTS files (e.g. imx6sl.dtsi for the fomer,
> > uniphier-pro4.dtsi for the latter). Is there a preferred way? I have a mild
> > preference for the first, as it would keep all 3 clocks in the same way. If I
> > go for the second, the clock driver would have to query its own node and the
> > parent node for the regmap.
>
> The first one is more natural...
>
> > AFAIU unrelated to syscon-reboot option 2 would give me the opportunity to
> > have separate clock and reset nodes and bindings and skip the aux bus, but
> > this would not be a correct representation of how the hardware works.
>
> ...for this reason!
> Do you actually need an aux bus here though? Since you have to add
> simple-mfd for your the syscon-reboot and simple-mfd is a real bus, can you
> set the reset controller up with an mfd_cell + devm_mfd_add_devices()
> instead?
Although, the /naming/ is more natural in the second case. "topcrm"
sounds like the actual name of this register region. And syscon@ for the
node name.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v5 6/8] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
From: sashiko-bot @ 2026-06-17 21:34 UTC (permalink / raw)
To: Frank.Li; +Cc: imx, Frank.Li, robh, conor+dt, devicetree
In-Reply-To: <20260617-imx8qxp_pcam-v5-6-7fa6c8e7fba7@nxp.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] NULL pointer dereference when enabling or disabling streams on an unconnected sink pad.
- [High] Missing format bounds clamping causes integer underflow and massive hardware width configuration.
- [Medium] Missing `V4L2_SUBDEV_FL_STREAMS` flag breaks V4L2 format and routing state initialization.
- [Medium] Reversed pad direction arguments in `v4l2_subdev_state_xlate_streams` leads to improper stream mask mapping.
- [Low] Unconditional overwrite of hardware configuration variable `val` renders `switch` statement dead code.
--
commit 6eecb6f34887a526e9a659dcd6d0a03e320f98ab
Author: Alice Yuan <alice.yuan@nxp.com>
media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
Add a V4L2 sub-device driver for the CPI controller found on i.MX8QXP,
i.MX8QM, and i.MX93 SoCs. This controller supports parallel camera sensors
and enables image data capture through a parallel interface.
> diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
> new file mode 100644
> index 0000000000000..00f5d5f47644b
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> @@ -0,0 +1,614 @@
[ ... ]
> +static void imx_cpi_config_ctrl_reg1(struct imx_cpi_device *pcpidev,
> + const struct v4l2_mbus_framefmt *format)
> +{
> + const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> + u32 pixel_width;
> + u32 vsync_pulse;
> + u32 val;
> +
> + pixel_width = format->width - 1;
> + vsync_pulse = format->width << 1;
> +
> + switch (pcpidev->pdata->version) {
> + case PI_V1:
> + val = CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
> + CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse);
> + break;
> + default:
> + val = 0; /* Never happen */
> + }
> +
> + val = CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
> + CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse);
[Severity: Low]
Is the unconditional assignment to val here intentional? It immediately
overwrites the val computed inside the switch statement above, effectively
rendering the switch block as dead code and dropping any future hardware
version configurations.
> + writel(val, pcpidev->regs + pdata->interface_ctrl_reg1);
> +}
[ ... ]
> +static int imx_cpi_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *sdformat)
> +{
> + struct imx_cpi_pix_format const *pcpidev_fmt;
> + struct v4l2_mbus_framefmt *fmt;
> +
> + /*
> + * The Parallel cpi can't transcode in any way, the source format
> + * can't be modified.
> + */
> + if (sdformat->pad == V4L2_SUBDEV_1TO1_PADS_SOURCE)
> + return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
> +
> + pcpidev_fmt = find_imx_cpi_format(sdformat->format.code);
> + if (!pcpidev_fmt)
> + pcpidev_fmt = &imx_cpi_formats[0];
> +
> + fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad);
> +
> + fmt->code = pcpidev_fmt->code;
> + fmt->width = sdformat->format.width;
> + fmt->height = sdformat->format.height;
[Severity: High]
Does this code need bounds checking for width and height? If a malicious
application provides a width of 0, it is copied directly to the format
structure here.
Later in imx_cpi_config_ctrl_reg1(), the width is used like this:
pixel_width = format->width - 1;
which would underflow to 0xffffffff, causing an extremely large dimension to
be written to the hardware configuration.
> + fmt->field = V4L2_FIELD_NONE;
> + fmt->colorspace = sdformat->format.colorspace;
[ ... ]
> +static int imx_cpi_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> + struct media_pad *sink_pad, *remote_pad;
> + struct device *dev = pcpidev->dev;
> + struct v4l2_subdev *remote_sd;
> + u64 mask;
> + int ret;
> +
> + sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
> + remote_pad = media_pad_remote_pad_first(sink_pad);
> + remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
[Severity: High]
Could this cause a NULL pointer dereference if the subdevice is not connected?
media_pad_remote_pad_first() returns NULL if no link is enabled on the sink
pad. If userspace sends VIDIOC_SUBDEV_S_STREAM directly to the unconnected
device node, remote_pad->entity would trigger a panic.
> +
> + mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
> + V4L2_SUBDEV_1TO1_PADS_SOURCE,
> + &streams_mask);
[Severity: Medium]
Are the pad arguments reversed here?
When disable_streams is called, streams_mask originates from the pad passed
to the operation (the SOURCE pad). To find the remote mask, it should
translate from SOURCE to SINK.
Should this be passing V4L2_SUBDEV_1TO1_PADS_SOURCE as the second argument
and V4L2_SUBDEV_1TO1_PADS_SINK as the third?
> +
> + ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
> + if (ret)
[ ... ]
> +static int imx_cpi_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> + const struct imx_cpi_pix_format *pcpidev_fmt;
> + const struct v4l2_mbus_framefmt *format;
> + struct media_pad *sink_pad, *remote_pad;
> + struct device *dev = pcpidev->dev;
> + struct v4l2_subdev *remote_sd;
> + u64 mask;
> + int ret;
> +
> + sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
> + remote_pad = media_pad_remote_pad_first(sink_pad);
> + remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
[Severity: High]
Does this have the same NULL pointer dereference issue on unconnected links as
imx_cpi_disable_streams()?
> +
> + mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
> + V4L2_SUBDEV_1TO1_PADS_SOURCE,
> + &streams_mask);
[Severity: Medium]
Similarly, do the pad arguments need to be swapped here to correctly
translate from SOURCE to SINK?
> +
> + format = v4l2_subdev_state_get_format(state, V4L2_SUBDEV_1TO1_PADS_SINK);
[ ... ]
> +static int imx_cpi_probe(struct platform_device *pdev)
> +{
> + struct imx_cpi_device *pcpidev;
> + struct device *dev = &pdev->dev;
> + struct v4l2_subdev *sd;
> + int ret;
[ ... ]
> + sd->internal_ops = &imx_cpi_internal_ops;
> + snprintf(sd->name, sizeof(sd->name), "parallel-%s",
> + dev_name(pcpidev->dev));
> +
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
[Severity: Medium]
Should V4L2_SUBDEV_FL_STREAMS also be set here?
Without this flag, the V4L2 core falls back to allocating the legacy
state->pads array. This causes the routing and stream configuration
set up by v4l2_subdev_set_routing_with_fmt() in imx_cpi_init_state()
to be ignored, which leaves the initial format reporting uninitialized.
> +
> + sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> + sd->entity.ops = &imx_cpi_entity_ops;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx8qxp_pcam-v5-0-7fa6c8e7fba7@nxp.com?part=6
^ permalink raw reply
* Re: [PATCH v4 10/10] Input: cap11xx - add support for CAP1114
From: sashiko-bot @ 2026-06-17 21:25 UTC (permalink / raw)
To: Jun Yan; +Cc: dmitry.torokhov, devicetree, linux-input, conor+dt, robh
In-Reply-To: <20260617150318.753148-11-jerrysteve1101@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds write in `cap11xx_i2c_probe` due to unvalidated Device Tree keycodes.
--
commit 42cef688f767435c4da8af0dd519be20d4164735
Author: Jun Yan <jerrysteve1101@gmail.com>
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.
This commit adds support for the CAP1114 touch sensor, updating the driver
to handle up to 14 channels and 11 LED outputs. It introduces logic to
read from a secondary status register to support the expanded channel count
and configures grouped sensors as independent inputs.
> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 275eb79a7193..865c58533eb9 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c
[ ... ]
> @@ -292,6 +307,17 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
> of_property_read_u32_array(node, "linux,keycodes",
> priv->keycodes, priv->model->num_channels);
[Severity: High]
This is a pre-existing issue, but I noticed that these keycodes are read
directly from the Device Tree into the priv->keycodes array without any
bounds validation.
Later during initialization in cap11xx_i2c_probe(), the driver loops over
these keycodes and sets bits using an unchecked macro:
__set_bit(priv->keycodes[i], priv->idev->keybit);
If an exceptionally large keycode (greater than KEY_MAX) is provided via a
Device Tree overlay, could this cause an out-of-bounds write on the
priv->idev->keybit array and corrupt adjacent heap memory? Should we verify
that each keycode read from the Device Tree is less than or equal to KEY_MAX?
> +
> + /*
> + * CAP1114 needs dedicated configuration to split
> + * grouped sensors into independent inputs.
> + */
> + if (priv->model->has_grouped_sensors) {
> + error = regmap_set_bits(priv->regmap, CAP1114_REG_CONFIG2,
> + CAP1114_REG_CONFIG2_VOL_UP_DOWN);
> + if (error)
> + return error;
> + }
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617150318.753148-1-jerrysteve1101@gmail.com?part=10
^ permalink raw reply
* Re: [PATCH RFC v4 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset bindings
From: Conor Dooley @ 2026-06-17 21:23 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <-l2OM6P0RNSYRQfOSObOyw@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]
On Wed, Jun 17, 2026 at 08:47:53PM +0300, Stefan Dösinger wrote:
> Am Mittwoch, 17. Juni 2026, 19:08:02 Ostafrikanische Zeit schrieb Conor
> Dooley:
> > On Tue, Jun 16, 2026 at 11:26:21PM +0300, Stefan Dösinger wrote:
>
> > > +F: Documentation/devicetree/zte,zx297520v3-*
> >
> > Sashiko complaint here looks valid.
>
> Yes, it is valid.
>
> It also brought up another concern in patch 12 that has an impact on this
> binding:
>
> Am Dienstag, 16. Juni 2026, 23:42:19 Ostafrikanische Zeit schrieb sashiko-
> bot@kernel.org:
> > issue(s) to consider: - [Low] The `syscon-reboot` node is incorrectly
> > defined at the root level using the explicitly deprecated `regmap`
> > property. --
>
> And indeed "regmap" is deprecated and I missed it somehow. As far as I
> understand, to put syscon-reboot as a child of the clock, I need to add the
> simple-mfd .compatible:
>
> Either
>
> topclk: clock-controller@13b000 {
> compatible = "zte,zx297520v3-topclk", "syscon", "simple-mfd";
> reg = <0x0013b000 0x400>;
> ...
> syscon-reboot {
> compatible = "syscon-reboot";
> regmap = <&topclk>;
> offset = <0x0>;
> mask = <0x1>;
> };
> };
>
> --- or ---
>
> something@13b000 {
> compatible = "zte,zx297520v3-topcrm", "syscon", "simple-mfd";
> reg = <0x0013b000 0x400>;
> ranges;
>
> topclk: clock-controller@0 {
> compatible = "zte,zx297520v3-topclk";
> ...
> }
>
> syscon-reboot {
> compatible = "syscon-reboot";
> offset = <0x0>;
> mask = <0x1>;
> };
> };
>
> I see both ways in existing DTS files (e.g. imx6sl.dtsi for the fomer,
> uniphier-pro4.dtsi for the latter). Is there a preferred way? I have a mild
> preference for the first, as it would keep all 3 clocks in the same way. If I
> go for the second, the clock driver would have to query its own node and the
> parent node for the regmap.
The first one is more natural...
> AFAIU unrelated to syscon-reboot option 2 would give me the opportunity to
> have separate clock and reset nodes and bindings and skip the aux bus, but
> this would not be a correct representation of how the hardware works.
...for this reason!
Do you actually need an aux bus here though? Since you have to add
simple-mfd for your the syscon-reboot and simple-mfd is a real bus, can you
set the reset controller up with an mfd_cell + devm_mfd_add_devices()
instead?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Conor Dooley @ 2026-06-17 21:17 UTC (permalink / raw)
To: Gerald Loacker
Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-phy, linux-arm-kernel,
linux-rockchip, linux-kernel, devicetree
In-Reply-To: <c34d4167-1a33-4e20-820c-735811b6a966@wolfvision.net>
[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]
On Wed, Jun 17, 2026 at 06:20:19PM +0200, Gerald Loacker wrote:
> Hi Conor,
>
> Am 17.06.2026 um 17:51 schrieb Conor Dooley:
> > On Wed, Jun 17, 2026 at 02:23:14PM +0200, Gerald Loacker wrote:
> >> Add support for the optional rockchip,clk-lane-phase device tree property
> >> to allow board-specific tuning of the clock lane sampling phase for
> >> improved signal integrity across supported data rates.
> >>
> >> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
> >> ---
> >> Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> index 03950b3cad08c..0d824d1511bc0 100644
> >> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> @@ -56,6 +56,13 @@ properties:
> >> description:
> >> Some additional phy settings are access through GRF regs.
> >>
> >> + rockchip,clk-lane-phase:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + minimum: 0
> >> + maximum: 7
> >> + description:
> >> + Clock lane sampling phase in 40 ps steps. The hardware default is 3.
> >
> > Can this instead become rockchip,clk-lane-phase-ps and be listed in the
> > actual unit?
> > With the -ps suffix, you can then drop the $ref.
> > The default should be listed as "default: 3" (or default: 120)
> >
> > pw-bot: changes-requested
> >
>
> Thanks for the suggestion.
>
> The phase setting is a hardware tap index (0–7) selecting a delay line
> position. The datasheet mentions “about 40 ps” per step, but this is not
> a calibrated or guaranteed value and may vary with PVT.
>
> Because of that, I’d prefer to keep the property as an index and
> document the approximate delay in the description:
>
> Clock lane sampling phase selection (hardware tap index 0–7). Each step
> corresponds to an approximately 40 ps delay as described in the hardware
> specification.
>
> This matches the hardware model more closely. Happy to adjust if needed.
>
Sure, I think that's fair.
> >> +
> >> required:
> >> - compatible
> >> - reg
> >>
> >> --
> >> 2.34.1
> >>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
From: Conor Dooley @ 2026-06-17 21:16 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: Marcelo Schmitt, linux-iio, devicetree, linux-kernel, jic23,
nuno.sa, Michael.Hennerich, dlechner, andy, robh, krzk+dt,
conor+dt, pop.ioan-daniel
In-Reply-To: <ajLV-O60lCdlIbXR@debian-BULLSEYE-live-builder-AMD64>
[-- Attachment #1: Type: text/plain, Size: 8059 bytes --]
On Wed, Jun 17, 2026 at 02:14:32PM -0300, Marcelo Schmitt wrote:
> On 06/17, Conor Dooley wrote:
> > On Tue, Jun 16, 2026 at 11:03:11PM -0300, Marcelo Schmitt wrote:
> > > Document how to describe LTC2378-20 and similar ADCs in device tree.
> > >
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > ---
> > > Change log v2 -> v3:
> > > - Re-added device tree fallback compatibles for LTC2378 chips, now with options
> > > to provide a single compatible string or a pair of single compatible string
> > > plus a fallback string to a slower sample rate spec in case a driver for the
> > > specific part is not found.
> > >
> > > .../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++++++++++++
> > > MAINTAINERS | 7 +
> > > 2 files changed, 167 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
> > > new file mode 100644
> > > index 000000000000..7d30a2cade8f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
> > > @@ -0,0 +1,160 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/adc/adi,ltc2378.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Analog Devices LTC2378 and similar Analog to Digital Converters
> > > +
> > > +maintainers:
> > > + - Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > +
> > > +description: |
> > > + Analog Devices LTC2378 series of ADCs.
> > > + Specifications can be found at:
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/233818fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/236416fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/236418f.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/236716fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/236718f.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/236816f.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/236818f.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/236918fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237016fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237616fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237618fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237620fb.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237716fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237718fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237720fb.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237816fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237818fa.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237820fb.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/237918fb.pdf
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/238016fb.pdf
> > > +
> > > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + # Single compatible string match.
> > > + - enum:
> > > + - adi,ltc2338-18
> > > + - adi,ltc2364-16
> > > + - adi,ltc2364-18
> > > + - adi,ltc2367-16
> > > + - adi,ltc2367-18
> > > + - adi,ltc2368-16
> > > + - adi,ltc2368-18
> > > + - adi,ltc2369-18
> > > + - adi,ltc2370-16
> > > + - adi,ltc2376-16
> > > + - adi,ltc2376-18
> > > + - adi,ltc2376-20
> > > + - adi,ltc2377-16
> > > + - adi,ltc2377-18
> > > + - adi,ltc2377-20
> > > + - adi,ltc2378-16
> > > + - adi,ltc2378-18
> > > + - adi,ltc2378-20
> > > + - adi,ltc2379-18
> > > + - adi,ltc2380-16
> > > +
> > > + # Low sample rate fallback for 16-bit unipolar sensors.
> > > + - items:
> > > + - enum:
> > > + - adi,ltc2370-16 # 2 MSPS
> > > + - adi,ltc2368-16 # 1 MSPS
> > > + - adi,ltc2367-16 # 500 kSPS
> > > + - const: adi,ltc2364-16 # fallback (250 kSPS)
> >
> > Your driver still matches on ltc2370-16, which makes me question the
> > value of these fallbacks. That said, the chip info struct contains no
> > information about sampling rate.
> The info about sample rate is added in patch 4.
Ah, I missed that.
> > What actually is the impact of the sample rate on the programming model?
> If the user tries to set a sample rate beyond the maximum supported, software
> can throw an error to indicate that.
I think you have misunderstood this. I was asking how the driver
interacts with the hardware. Do all devices come out of reset with the
minimum sampling rate? Or a per-device default rate? Do the faster
devices support all slower rates? If they do, do the registers have the
same meaning and the same value sets 2370-16 device to 1 MSPS as
2368-16.
> > Is there actually a benefit to matching on ltc2370-16, or can you just
> > match on the fallback?
> The benefit is telling software that it is safe to go up to 2 MSPS. It is also
> okay to match on the fallback, but software may restrain operation to a slower
> sample rate.
My whole review here is shit cos I didn't see patch 4, so the use of
fallbacks seems appropriate (provided your answers to my programming
model questions are sane!).
>
> With the code being proposed in this patch set, there is no benefit in matching
> the ltc2364-16 fallback if there is a faster ltc2370-16 device connected because
> device driver supports that. Though, other operating systems and/or platforms
> might not support all device variants and so the fallback might be useful.
>
> > +static const struct ltc2378_chip_info ltc2370_16_chip_info = {
> > + .name = "ltc2370-16",
> > + .resolution = 16,
> In a later patch ...
> + .max_sample_rate_hz = 2 * HZ_PER_MHZ,
> + .tconv_ns = 322,
> > + .bipolar = false,
> > +};
> >
> > +static const struct ltc2378_chip_info ltc2368_16_chip_info = {
> > + .name = "ltc2368-16",
> > + .resolution = 16,
> + .max_sample_rate_hz = HZ_PER_MHZ,
> + .tconv_ns = 527
> > + .bipolar = false,
> > +};
> >
> > +static const struct ltc2378_chip_info ltc2367_16_chip_info = {
> > + .name = "ltc2367-16",
> > + .resolution = 16,
> + .max_sample_rate_hz = 500 * HZ_PER_KHZ,
> + .tconv_ns = 1500,
> > + .bipolar = false,
> > +};
> >
> > +static const struct ltc2378_chip_info ltc2364_16_chip_info = {
> > + .name = "ltc2364-16",
> > + .resolution = 16,
> + .max_sample_rate_hz = 250 * HZ_PER_KHZ,
> + .tconv_ns = 3000,
> > + .bipolar = false,
> > +};
> >
> ...
> > > + # Low sample rate fallback for 20-bit bipolar sensors.
> > > + - items:
> > > + - enum:
> > > + - adi,ltc2378-20 # 1 MSPS
> > > + - adi,ltc2377-20 # 500 kSPS
> > > + - const: adi,ltc2376-20 # fallback (250 kSPS)
> >
> > I didn't check these, but I assume they are the same.
>
> Yes, except for small variations on input (unipolar/bipolar) and precision bits,
> these devices are pretty much equal.
What I meant here was that 2378-20, 2377-20 and 2376-20 would follow the
same pattern of having identical match data (pre patch 4).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 09/10] dt-bindings: input: microchip,cap11xx: Add CAP1114 support
From: sashiko-bot @ 2026-06-17 21:15 UTC (permalink / raw)
To: Jun Yan; +Cc: conor+dt, devicetree, robh, linux-input, dmitry.torokhov
In-Reply-To: <20260617150318.753148-10-jerrysteve1101@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Schema Validation Regression (Incomplete Constraints Update)
--
commit 74e08f50d786114a66be01956a59aeb6b7083fbc
Author: Jun Yan <jerrysteve1101@gmail.com>
dt-bindings: input: cap11xx: Add CAP1114 support
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support. Add the compatible string for CAP1114, add
its datasheet URL, update the maximum of LED channel reg, and add
constraint for linux,keycodes.
> diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> index b97e5b2735f1..2a37ac252c37 100644
> --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
[ ... ]
> @@ -140,7 +144,7 @@ properties:
> The number of entries must correspond to the number of channels.
>
> patternProperties:
> - "^led@[0-7]$":
> + "^led@[0-9a]$":
[Severity: Medium]
Does this global regex update also require an update to the disablement block
for hardware variants without LEDs?
The allOf block that disables LEDs for specific variants (like CAP1106,
CAP1203, etc.) still uses "^led@[0-7]$": false.
Because of this new global regex, wouldn't nodes like led@8, led@9,
and led@a now bypass the false constraint and be silently permitted on
devices that are supposed to have zero LEDs?
> type: object
> description: CAP11xx LEDs
> $ref: /schemas/leds/common.yaml#
> @@ -149,7 +153,7 @@ patternProperties:
> reg:
> description: LED channel number
> minimum: 0
> - maximum: 7
> + maximum: 10
>
> label: true
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617150318.753148-1-jerrysteve1101@gmail.com?part=9
^ permalink raw reply
* Re: [PATCH V13 8/9] iio: imu: inv_icm42607: Add Gyroscope to icm42607
From: Chris Morgan @ 2026-06-17 21:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Chris Morgan, linux-iio, andy, nuno.sa, dlechner, jic23,
jean-baptiste.maneyrol, linux-rockchip, devicetree, heiko,
conor+dt, krzk+dt, robh
In-Reply-To: <ajEhr0JIQwzRgoZa@ashevche-desk.local>
On Tue, Jun 16, 2026 at 01:13:03PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 15, 2026 at 12:25:51PM -0500, Chris Morgan wrote:
>
> > Add gyroscope functions to the icm42607 driver.
>
> ...
>
> > +int inv_icm42607_set_gyro_conf(struct inv_icm42607_state *st,
> > + struct inv_icm42607_sensor_conf *conf,
> > + unsigned int *sleep_ms)
> > +{
> > + struct inv_icm42607_sensor_conf *oldconf = &st->conf.gyro;
> > + unsigned int val;
> > + int ret;
> > +
> > + if (conf->mode < 0)
> > + conf->mode = oldconf->mode;
> > + if (conf->fs < 0)
> > + conf->fs = oldconf->fs;
> > + if (conf->odr < 0)
> > + conf->odr = oldconf->odr;
> > + if (conf->filter < 0)
> > + conf->filter = oldconf->filter;
>
> Same comment as per previous patch. But looking at this, can you rather have
> a helper that answers the below two questions? Something like
>
> void _assign_conf(..., bool *write_odr, bool *write_filter)
> {
> ...
> }
> EXPORT_...
>
> in the core driver? But wight both approaches and choose either existing one
> (as in this patch series) or what I suggested.
In the core driver, I was going to do something like this:
static void inv_icm42607_update_config(struct inv_icm42607_sensor_conf *conf,
struct inv_icm42607_sensor_conf *oldconf,
bool *config0, bool *config1)
{
if (conf->mode < 0)
conf->mode = oldconf->mode;
if (conf->fs < 0)
conf->fs = oldconf->fs;
if (conf->odr < 0)
conf->odr = oldconf->odr;
if (conf->filter < 0)
conf->filter = oldconf->filter;
if (conf->fs != oldconf->fs || conf->odr != oldconf->odr)
*config0 = true;
if (conf->filter != oldconf->filter)
*config1 = true;
}
So the step of copying stuff for sanity checking as well as checking
if I need to update config0 or config1 registers is handled here, and
I can simply check the values of config0 and config1 with an if to
determine if I need to call to write to the registers.
>
> > + if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> > + val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK,
> > + conf->fs);
> > + val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK,
> > + conf->odr);
> > + ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
> > + if (ret)
> > + return ret;
> > + oldconf->fs = conf->fs;
> > + oldconf->odr = conf->odr;
> > + }
> > +
> > + if (conf->filter != oldconf->filter) {
> > + val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK,
> > + conf->filter);
> > + ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
> > + INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
> > + if (ret)
> > + return ret;
> > + oldconf->filter = conf->filter;
> > + }
> > +
> > + return inv_icm42607_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
> > + st->conf.temp_en, sleep_ms);
> > +}
>
> ...
>
> > +{
> > + unsigned int odr;
> > + unsigned int i;
> > +
> > + guard(mutex)(&st->lock);
> > +
> > + odr = st->conf.gyro.odr;
> > +
> > + for (i = 5; i < ARRAY_SIZE(inv_icm42607_gyro_odr); ++i) {
>
> Same comment, why pre-increment?
I could not brain on this day, I had the dumb.
I'll fix it. :-)
>
> > + if (i == odr)
> > + break;
> > + }
> > + if (i >= ARRAY_SIZE(inv_icm42607_gyro_odr))
> > + return -EINVAL;
> > +
> > + *val = inv_icm42607_gyro_odr[i][0];
> > + *val2 = inv_icm42607_gyro_odr[i][1];
> > +
> > + return IIO_VAL_INT_PLUS_MICRO;
> > +}
> > +
> > +static int inv_icm42607_gyro_write_odr(struct iio_dev *indio_dev,
> > + int val, int val2)
> > +{
> > + struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> > + struct device *dev = regmap_get_device(st->map);
> > + unsigned int idx;
> > + struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
> > + int ret;
> > +
> > + for (idx = 5; idx < ARRAY_SIZE(inv_icm42607_gyro_odr); ++idx) {
>
> Ditto.
>
> > + if (val == inv_icm42607_gyro_odr[idx][0] &&
> > + val2 == inv_icm42607_gyro_odr[idx][1])
> > + break;
> > + }
> > + if (idx >= ARRAY_SIZE(inv_icm42607_gyro_odr))
> > + return -EINVAL;
> > +
> > + conf.odr = idx;
> > +
> > + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> > + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> > + if (ret)
> > + return ret;
> > +
> > + guard(mutex)(&st->lock);
> > +
> > + return inv_icm42607_set_gyro_conf(st, &conf, NULL);
> > +}
>
> Can be some of the code deduplicated between gyro and accel?
Probably a fair amount, but the deduplication will likely need to be
undone somewhat if we get buffer, WoM or apex support added back
(I don't have any devices with such functionality, so if anyone will
do it then it won't be me). I can refactor more if you want, or we
can keep it split like this to make it easy if someone else wants to
tackle the buffers/IRQs stuff later? Your call.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Thank you,
Chris
^ permalink raw reply
* Re: [PATCH v9 5/5] iio: adc: versal-sysmon: add oversampling support
From: sashiko-bot @ 2026-06-17 21:07 UTC (permalink / raw)
To: Salih Erim; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260617180147.3370346-6-salih.erim@amd.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Software cache for oversampling ratios is unconditionally initialized to 1, potentially causing desynchronization with the actual hardware configuration.
--
commit 81ca0dc5aa386dec0337a8e068370935d45ea78e
Author: Salih Erim <salih.erim@amd.com>
iio: adc: versal-sysmon: add oversampling support
This commit adds support for reading and writing the oversampling ratio
through the IIO oversampling_ratio attribute. The hardware supports averaging
2, 4, 8, or 16 samples, plus a ratio of 1 for no averaging.
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index 19a8edd89198c..16f474c1bffae 100644
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
[ ... ]
> @@ -832,6 +981,8 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
>
> sysmon = iio_priv(indio_dev);
> sysmon->regmap = regmap;
> + sysmon->temp_oversampling = 1;
> + sysmon->supply_oversampling = 1;
[Severity: Medium]
Could this software cache initialization become desynchronized with the actual
hardware configuration?
The oversampling variables are initialized to 1 unconditionally, without
reading the SYSMON_CONFIG hardware register or actively writing this default
value down to the hardware.
If the bootloader or a previous kernel instance (such as through kexec)
configured an oversampling ratio other than 1, will the hardware continue
averaging samples while userspace incorrectly reads '1' from the IIO
oversampling ratio attribute?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617180147.3370346-1-salih.erim@amd.com?part=5
^ permalink raw reply
* Re: [PATCH v5 2/8] media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration
From: sashiko-bot @ 2026-06-17 21:06 UTC (permalink / raw)
To: Frank.Li; +Cc: conor+dt, imx, Frank.Li, robh, devicetree
In-Reply-To: <20260617-imx8qxp_pcam-v5-2-7fa6c8e7fba7@nxp.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant cleanup and misguided teardown of `sd->subdev_notifier`.
--
commit ebb79cbbc440da4b8d1a35f25ce9168deff7467d
Author: Frank Li <Frank.Li@nxp.com>
media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration
This commit introduces a common helper library to encapsulate registration,
media entity initialization, and cleanup for 1-to-1 V4L2 subdevices,
reducing boilerplate in simple bridge and converter drivers.
> diff --git a/drivers/media/v4l2-core/v4l2-1to1.c b/drivers/media/v4l2-core/v4l2-1to1.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-1to1.c
[ ... ]
> +void media_async_subdev_1to1_cleanup(struct v4l2_subdev_1to1 *sd_1to1)
> +{
> + struct v4l2_subdev *sd = &sd_1to1->sd;
> +
> + v4l2_async_unregister_subdev(sd);
> + v4l2_subdev_cleanup(sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_async_nf_unregister(sd->subdev_notifier);
> + v4l2_async_nf_cleanup(sd->subdev_notifier);
> +
> + kfree(sd->subdev_notifier);
[Severity: Low]
Does this code perform redundant cleanups?
Looking at v4l2_async_unregister_subdev(), it already handles unregistering,
cleaning up, freeing, and setting sd->subdev_notifier to NULL:
v4l2_async_unregister_subdev(struct v4l2_subdev *sd) {
...
__v4l2_async_nf_unregister(sd->subdev_notifier);
__v4l2_async_nf_cleanup(sd->subdev_notifier);
kfree(sd->subdev_notifier);
sd->subdev_notifier = NULL;
...
}
Since sd->subdev_notifier is NULL after v4l2_async_unregister_subdev()
returns, are the subsequent calls to v4l2_async_nf_unregister(),
v4l2_async_nf_cleanup(), and kfree() necessary here in
media_async_subdev_1to1_cleanup()?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx8qxp_pcam-v5-0-7fa6c8e7fba7@nxp.com?part=2
^ permalink raw reply
* Re: [PATCH v9 4/5] iio: adc: versal-sysmon: add threshold event support
From: sashiko-bot @ 2026-06-17 21:01 UTC (permalink / raw)
To: Salih Erim; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260617180147.3370346-5-salih.erim@amd.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Integer Overflow and Missing Bit Masking in Threshold Calculations
- [High] Supply Voltage Threshold Events Permanently Disabled on Trigger
- [High] Kernel Panic from Sleeping in Atomic Context on I2C Devices
- [High] Race Condition Desynchronizes Hardware Interrupt State
--
commit 0d7222f55024a692e00cced200dc17d0764c1060
Author: Salih Erim <salih.erim@amd.com>
iio: adc: versal-sysmon: add threshold event support
Adds threshold event support for temperature and supply voltage channels.
It uses hysteresis mode for temperature and provides per-channel alarm
enable for supply voltages. The interrupt handler masks active threshold
interrupts and schedules a delayed worker to poll.
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index e0b9b3d2b6d13..19a8edd89198c 100644
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
[ ... ]
> @@ -52,6 +101,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;
> +}
[Severity: High]
Can this left shift overflow on negative or large sysfs values? Unlike the
voltage path, this function also seems to lack value clamping and a (u16)
bitmask. Since sysmon_write_event_value() writes this raw 32-bit value
directly to SYSMON_TEMP_TH_UP, could it overwrite reserved hardware bits
with sign-extended negative values like 0xFFFFEC00?
> +
> static void sysmon_supply_rawtoprocessed(int raw_data, int *val)
> {
> int mantissa, format, exponent;
> @@ -69,6 +128,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;
[Severity: High]
Can this 32-bit multiplication overflow on large sysfs inputs? If scale is up
to 65536, a large val could cause an integer overflow before the division.
> +
> + if (format)
> + tmp = clamp(tmp, S16_MIN, S16_MAX);
> + else
> + tmp = clamp(tmp, 0, U16_MAX);
> +
> + *raw_data = (u16)tmp;
> +}
[ ... ]
> +static int sysmon_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + bool state)
> +{
[ ... ]
> + case IIO_TEMP:
> + if (state) {
> + ret = regmap_write(sysmon->regmap, SYSMON_IER, mask);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask &= ~mask;
> + } else {
> + ret = regmap_write(sysmon->regmap, SYSMON_IDR, mask);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask |= mask;
> + }
> + return 0;
[Severity: High]
Is there a race condition here if sysmon_unmask_worker() runs concurrently?
By writing to SYSMON_IDR before acquiring sysmon->irq_lock to update
sysmon->temp_mask, the concurrent worker might read the old temp_mask (where
the bit is still 0/enabled). Could the worker then write to SYSMON_IER in
sysmon_unmask_temp(), effectively re-enabling the interrupt in hardware
immediately after userspace administratively disabled it?
[ ... ]
> +static int sysmon_handle_event(struct iio_dev *indio_dev, u32 event)
> +{
> + u32 alarm_flag_offset = SYSMON_ALARM_FLAG + event * SYSMON_REG_STRIDE;
> + u32 alarm_reg_offset = SYSMON_ALARM_REG + event * SYSMON_REG_STRIDE;
> + struct sysmon *sysmon = iio_priv(indio_dev);
> + unsigned long alarm_flag_reg;
> + unsigned int reg_val;
> + u32 address, bit;
> + int ret;
> +
> + switch (event) {
[ ... ]
> + case SYSMON_BIT_ALARM0:
> + case SYSMON_BIT_ALARM1:
> + case SYSMON_BIT_ALARM2:
> + case SYSMON_BIT_ALARM3:
> + case SYSMON_BIT_ALARM4:
> + ret = regmap_read(sysmon->regmap, alarm_flag_offset, ®_val);
> + if (ret)
> + return ret;
> +
> + alarm_flag_reg = reg_val;
> +
> + for_each_set_bit(bit, &alarm_flag_reg, SYSMON_ALARM_BITS_PER_REG) {
> + address = bit + SYSMON_ALARM_BITS_PER_REG * event;
> + sysmon_push_event(indio_dev, address);
> + ret = regmap_clear_bits(sysmon->regmap, alarm_reg_offset, BIT(bit));
[Severity: High]
Does clearing this bit permanently disable the supply voltage hardware alarm?
The alarm_reg_offset here points to SYSMON_ALARM_REG, which
sysmon_write_alarm_config() uses as the alarm configuration/enable register.
Since there is no mechanism to re-enable voltage alarms later, does this turn
continuous threshold events into one-shot events?
> + if (ret)
> + return ret;
> + }
> +
> + return regmap_write(sysmon->regmap, alarm_flag_offset, alarm_flag_reg);
> +
> + default:
> + return -EINVAL;
> + }
> +}
[ ... ]
> +static void sysmon_unmask_worker(struct work_struct *work)
> +{
> + struct sysmon *sysmon =
> + container_of(work, struct sysmon, sysmon_unmask_work.work);
> + unsigned int isr;
> +
> + /*
> + * If the ISR read fails, skip processing to avoid acting
> + * on undefined data.
> + */
> + scoped_guard(spinlock_irq, &sysmon->irq_lock) {
> + if (regmap_read(sysmon->regmap, SYSMON_ISR, &isr))
> + break;
[Severity: High]
Can this cause a "scheduling while atomic" panic on I2C devices?
The driver takes a spinlock with interrupts disabled via spinlock_irq,
but then calls regmap_read() which uses a sleeping mutex on I2C buses.
A similar sequence occurs in the hardirq handler sysmon_iio_irq().
> + regmap_write(sysmon->regmap, SYSMON_ISR, isr);
> + sysmon_unmask_temp(sysmon, isr);
> + }
[ ... ]
> +static int sysmon_init_interrupt(struct sysmon *sysmon,
> + struct device *dev,
> + struct iio_dev *indio_dev,
> + int irq)
> +{
> + unsigned int imr;
> + int ret;
> +
> + /* Events not supported without IRQ (e.g. I2C path) */
> + if (!irq)
> + return 0;
> +
> + INIT_DELAYED_WORK(&sysmon->sysmon_unmask_work, sysmon_unmask_worker);
> +
> + ret = regmap_read(sysmon->regmap, SYSMON_IMR, &imr);
> + if (ret)
> + return ret;
> + sysmon->temp_mask = imr & SYSMON_TEMP_INTR_MASK;
> +
> + ret = devm_request_irq(dev, irq, sysmon_iio_irq, 0, "sysmon-irq", indio_dev);
[Severity: High]
Should we verify that the regmap doesn't sleep before requesting the IRQ?
If a user provides an interrupts property for an I2C-connected device in
the Device Tree, it looks like this will register sysmon_iio_irq() as a
hardirq handler and trigger the atomic sleep violations mentioned above.
> + if (ret)
> + return ret;
> +
> + return devm_add_action_or_reset(dev, sysmon_disable_interrupts, sysmon);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617180147.3370346-1-salih.erim@amd.com?part=4
^ permalink raw reply
* Re: [PATCH v4 07/10] Input: cap11xx - refactor code for better CAP1114 support.
From: sashiko-bot @ 2026-06-17 20:57 UTC (permalink / raw)
To: Jun Yan; +Cc: conor+dt, devicetree, dmitry.torokhov, linux-input, robh
In-Reply-To: <20260617150318.753148-8-jerrysteve1101@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Bitmask truncation in `cap11xx_led_set` causing silent failure for LEDs >= 8.
- [High] Regmap cache staleness due to missing volatile register definition for the dynamic `sensor_input_reg_base`.
Pre-existing issues:
- [High] Buffer overflow and Fortify Source panic when reading `calib_sensitivities` for devices with more than 8 channels.
--
commit 837f207153012dfc0860b6d46ad5d3ae4f7b3004
Author: Jun Yan <jerrysteve1101@gmail.com>
Extend cap11xx_hw_model structure to support CAP1114
This patch extends the cap11xx_hw_model structure to support CAP1114
with different register offsets and hardware characteristics.
> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 1db4a9090705..0f19ee036e78 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c
[ ... ]
> @@ -211,8 +215,8 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
> }
>
> if (!of_property_read_u32_array(node, "microchip,input-threshold",
> - priv->thresholds, priv->model->num_channels)) {
> + priv->thresholds, priv->model->num_sensor_thresholds)) {
> - for (i = 0; i < priv->model->num_channels; i++) {
> + for (i = 0; i < priv->model->num_sensor_thresholds; i++) {
> if (priv->thresholds[i] > 127) {
> dev_err(dev, "Invalid input-threshold value %u\n",
> priv->thresholds[i]);
[Severity: High]
This is a pre-existing issue, but looking at cap11xx_init_keys(), could
reading "microchip,calib-sensitivity" overflow the calib_sensitivities array
for devices with more than 8 channels?
In cap11xx_init_keys():
} else if (!of_property_read_u32_array(node, "microchip,calib-sensitivity",
priv->calib_sensitivities,
priv->model->num_channels)) {
The calib_sensitivities array is statically allocated to 8 elements in
struct cap11xx_priv. For models with more than 8 channels (like the CAP1114
which has 14), could this cause a buffer overflow into the keycodes array
and trigger a kernel panic if FORTIFY_SOURCE is enabled?
[ ... ]
> @@ -308,7 +314,7 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
> if (ret < 0)
> goto out;
>
> - ret = regmap_read(priv->regmap, CAP11XX_REG_SENSOR_INPUT, &status);
> + ret = regmap_read(priv->regmap, priv->model->sensor_input_reg_base, &status);
> if (ret < 0)
> goto out;
[Severity: High]
Will this result in reading stale cached values instead of the actual
hardware state?
The interrupt handler now dynamically reads from sensor_input_reg_base.
However, cap11xx_volatile_reg() still hardcodes CAP11XX_REG_SENSOR_INPUT as
a volatile register.
Since the new dynamic base register (and the subsequent register for devices
with more than 8 channels) is no longer marked volatile, will the regmap
cache return stale data and break input events for these models?
[ ... ]
> @@ -362,7 +368,7 @@ static int cap11xx_led_set(struct led_classdev *cdev,
> * 0 (OFF) and 1 (ON).
> */
> return regmap_update_bits(priv->regmap,
> - CAP11XX_REG_LED_OUTPUT_CONTROL,
> + priv->model->led_output_control_reg_base,
> BIT(led->reg),
> value ? BIT(led->reg) : 0);
> }
[Severity: High]
Will this bitmask be truncated for LEDs with an index of 8 or higher?
The bitmask passed to regmap_update_bits() is BIT(led->reg). Since models
like CAP1114 support more than 8 LEDs, and the register operations are 8-bit,
BIT(led->reg) will exceed 8 bits for higher-index LEDs.
The initialization code in cap11xx_init_leds() caps the LED clearance mask to
8 bits:
GENMASK(min(num_leds, 8) - 1, 0)
Does cap11xx_led_set() also need an update to handle devices with more than
8 LEDs to avoid silently failing?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617150318.753148-1-jerrysteve1101@gmail.com?part=7
^ permalink raw reply
* Re: [PATCH v5 1/8] media: v4l2-fwnode: Extract common helper __v4l2_async_register_subdev_fwnode()
From: sashiko-bot @ 2026-06-17 20:51 UTC (permalink / raw)
To: Frank.Li; +Cc: imx, robh, devicetree, Frank.Li, conor+dt
In-Reply-To: <20260617-imx8qxp_pcam-v5-1-7fa6c8e7fba7@nxp.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message incorrectly states the function from which the common helper is extracted.
--
commit 1d02bf2bbb834a499240f695cd83dc1f1f8db5fb
Author: Frank Li <Frank.Li@nxp.com>
media: v4l2-fwnode: Extract common helper __v4l2_async_register_subdev_fwnode()
This commit refactors the subdev registration logic to extract a common
helper, __v4l2_async_register_subdev_fwnode(). It allows the notifier
parsing function to be passed as a parameter so that different fwnode
parsing implementations can reuse the same registration code.
> Extract __v4l2_async_register_subdev_fwnode() from
> __media_async_register_subdev_1to1() and make the notifier parsing
> function a parameter.
[Severity: Medium]
Is this function name correct? The commit message states the helper is
extracted from __media_async_register_subdev_1to1(), but the diff shows
the logic is actually being extracted from
__v4l2_async_register_subdev_sensor().
Should the commit message be updated to match the actual function being
modified in the code?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx8qxp_pcam-v5-0-7fa6c8e7fba7@nxp.com?part=1
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: arm: xen: Convert to DT schema
From: sashiko-bot @ 2026-06-17 20:51 UTC (permalink / raw)
To: Tejas Mutalikdesai; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260617185518.43500-1-tejasmutalikdesai@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The `$nodename` pattern requires a unit address, which breaks Linux's hardcoded Xen node detection.
--
commit e4969fb4c5852849c9ecbfff9b63719a0d10dc05
Author: Tejas Mutalikdesai <tejasmutalikdesai@gmail.com>
dt-bindings: arm: xen: Convert to DT schema
Convert the Xen ARM device tree binding documentation from the legacy
plain-text format to the DT schema format.
> diff --git a/Documentation/devicetree/bindings/arm/xen.yaml b/Documentation/devicetree/bindings/arm/xen.yaml
> new file mode 100644
> index 0000000000000..a22e950566c23
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/xen.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
[ ... ]
> +description:
> + Xen ARM virtual platforms shall have a top-level "hypervisor" node with
> + the properties defined below.
> +
> +properties:
> + $nodename:
> + pattern: "^hypervisor@[0-9a-f]+$"
[Severity: Critical]
Does enforcing a unit address here break existing kernel expectations?
The Linux kernel seems to strictly expect the node to be named exactly
"hypervisor" without a unit address.
In arch/arm/xen/enlighten.c:fdt_find_hyper_node(), the code uses:
if (depth != 1 || strcmp(uname, "hypervisor") != 0)
And in drivers/firmware/efi/fdtparams.c:dt_params[], the path is hardcoded:
.path = "/hypervisor/uefi",
If a hypervisor or devicetree generator updates its output to comply with this
new schema and adds a unit address (e.g., hypervisor@b0000000), won't the
kernel silently fail to detect Xen and its UEFI runtime services?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617185518.43500-1-tejasmutalikdesai@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v7 4/4] arm64: tegra: Reorder reg and reg-names to match bindings
From: sashiko-bot @ 2026-06-17 20:50 UTC (permalink / raw)
To: Thierry Reding; +Cc: conor+dt, linux-pci, devicetree, robh
In-Reply-To: <20260617-tegra264-pcie-v7-4-eae7ae964629@nvidia.com>
> From: Thierry Reding <treding@nvidia.com>
>
> The ECAM region cannot be the first entry in the "reg" property, because
> in that case the unit-address wouldn't match the first entry. The order
> of the nodes can also not be changed to match the ECAM entry because the
> ECAM region is global and outside of any of the control busses.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-tegra264-pcie-v7-0-eae7ae964629@nvidia.com?part=4
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: clock: renesas,r9a09g077/87: Add PCLKRTC clock ID
From: Lad, Prabhakar @ 2026-06-17 20:49 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
linux-renesas-soc, linux-clk, devicetree, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CAMuHMdXQ3S37L7JAJ42Bag0UPa15Q7n-nNCcbV9FXTtTs4aihQ@mail.gmail.com>
Hi Geert,
Thank you for the review.
On Wed, Jun 17, 2026 at 11:19 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Mon, 15 Jun 2026 at 16:40, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add the peripheral clock ID definition for the Real-Time Clock (PCLKRTC)
> > on the Renesas RZ/T2H (R9A09G077) and RZ/N2H (R9A09G087) SoCs.
> >
> > Note that the LCDC_CLKD clock is utilized as the operating clock source
>
> PCLKRTC
>
Oops my bad!
> > for the RTC IP.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in a branch shared by renesas-clk and renesas-dts
> with the above fixed.
>
Thank you for taking care of this issue.
Cheers,
Prabhakar
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v7 3/4] PCI: tegra: Add Tegra264 support
From: sashiko-bot @ 2026-06-17 20:48 UTC (permalink / raw)
To: Thierry Reding; +Cc: robh, linux-pci, devicetree, conor+dt
In-Reply-To: <20260617-tegra264-pcie-v7-3-eae7ae964629@nvidia.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unbinding the driver causes a NULL pointer dereference crash if the PCIe link was down during probe.
- [High] Kernel panic during system suspend if the PCIe link failed to come up.
- [Medium] The runtime PM usage counter is leaked if `pm_runtime_get_sync()` fails.
--
commit 606e8e105f99594322aaccfdb36dcac03cb19d29
Author: Thierry Reding <treding@nvidia.com>
PCI: tegra: Add Tegra264 support
Add a driver for the PCIe controller found on NVIDIA Tegra264 SoCs. The
driver is very small, with its main purpose being to set up the address
translation registers and then creating a standard PCI host using ECAM.
> diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
> new file mode 100644
> index 0000000000000..e2d295ea4403f
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-tegra264.c
[ ... ]
> +static int tegra264_pcie_probe(struct platform_device *pdev)
> +{
[ ... ]
> + err = devm_pm_runtime_set_active_enabled(dev);
> + if (err < 0) {
> + dev_err_probe(dev, err, "failed to enable runtime PM\n");
> + goto put_bpmp;
> + }
> +
> + err = pm_runtime_get_sync(dev);
> + if (err < 0) {
> + dev_err_probe(dev, err, "failed to power on device\n");
> + goto put_bpmp;
[Severity: Medium]
According to the runtime PM API, pm_runtime_get_sync() increments the usage
counter even if it returns an error. If it fails here, we jump to put_bpmp,
skipping the put_pm label which calls pm_runtime_put_sync(). Does this leak
the usage counter and prevent the device from ever entering runtime suspend?
> + }
> +
> + /* sanity check that programmed ranges match what's in DT */
> + if (!tegra264_pcie_check_ranges(pdev)) {
> + err = -EINVAL;
> + goto put_pm;
> + }
[ ... ]
> + tegra264_pcie_init(pcie);
> +
> + if (!pcie->link_up)
> + return 0;
> +
> + err = pci_host_probe(bridge);
> + if (err < 0) {
> + dev_err_probe(dev, err, "failed to register host\n");
> + goto free_ecam;
> + }
> +
> + return 0;
> +
> +free_ecam:
> + pci_ecam_free(pcie->cfg);
> +put_pm:
> + pm_runtime_put_sync(dev);
> +put_bpmp:
> + tegra_bpmp_put(pcie->bpmp);
> +
> + return err;
> +}
> +
> +static void tegra264_pcie_remove(struct platform_device *pdev)
> +{
> + struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
> +
> + /*
> + * If we undo tegra264_pcie_init() then link goes down and need
> + * controller reset to bring up the link again. Remove intention is
> + * to clean up the root bridge and re-enumerate during bind.
> + */
> + pci_lock_rescan_remove();
> + pci_stop_root_bus(pcie->bridge->bus);
> + pci_remove_root_bus(pcie->bridge->bus);
[Severity: High]
If the link is down during probe, tegra264_pcie_probe() returns 0 without
calling pci_host_probe(), leaving pcie->bridge->bus as NULL. When the
driver is later unbound, tegra264_pcie_remove() unconditionally passes
pcie->bridge->bus to pci_stop_root_bus() and pci_remove_root_bus().
Will this cause a NULL pointer dereference? Is a check needed to ensure the
bus exists before tearing it down?
> + pci_unlock_rescan_remove();
> +
> + pm_runtime_put_sync(&pdev->dev);
> + tegra_bpmp_put(pcie->bpmp);
> + pci_ecam_free(pcie->cfg);
> +}
> +
> +static int tegra264_pcie_suspend(struct device *dev)
> +{
> + struct tegra264_pcie *pcie = dev_get_drvdata(dev);
> + int err;
> +
> + tegra264_pcie_deinit(pcie);
[Severity: High]
If the link fails to come up in tegra264_pcie_init() and the port isn't
hotplug-capable, the driver powers off the controller and sets
pcie->link_up = false.
If the system is later suspended, tegra264_pcie_suspend() unconditionally
calls tegra264_pcie_deinit(), which attempts to read from the xtl register
space:
static void tegra264_pcie_deinit(struct tegra264_pcie *pcie)
{
...
value = readl(pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
Will this hardware access to a powered-off controller cause a kernel panic?
Should we check pcie->link_up before calling tegra264_pcie_deinit(),
similar to what is done in tegra264_pcie_resume()?
> +
> + if (pcie->wake_gpio && device_may_wakeup(dev)) {
> + err = enable_irq_wake(pcie->wake_irq);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-tegra264-pcie-v7-0-eae7ae964629@nvidia.com?part=3
^ permalink raw reply
* Re: [PATCH v9 3/5] iio: adc: versal-sysmon: add I2C driver
From: sashiko-bot @ 2026-06-17 20:47 UTC (permalink / raw)
To: Salih Erim; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260617180147.3370346-4-salih.erim@amd.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Non-atomic I2C read operation splits the command write and data read into two distinct I2C transactions, risking state corruption from interleaved bus traffic.
--
commit ae0266701d933c996d4059785f81513fb71dc331
Author: Salih Erim <salih.erim@amd.com>
iio: adc: versal-sysmon: add I2C driver
This commit adds an I2C transport driver for the Versal SysMon block. It
provides an I2C slave interface that allows an external master to read
voltage and temperature measurements through the same register map used by
the MMIO path.
> diff --git a/drivers/iio/adc/versal-sysmon-i2c.c b/drivers/iio/adc/versal-sysmon-i2c.c
> new file mode 100644
> index 0000000000000..e9a7629159ab1
> --- /dev/null
> +++ b/drivers/iio/adc/versal-sysmon-i2c.c
[ ... ]
> +static int sysmon_i2c_reg_read(void *context, unsigned int reg,
> + unsigned int *val)
> +{
> + struct i2c_client *client = context;
> + u8 write_buf[8] = { };
> + u8 read_buf[4];
> + int ret;
> +
> + put_unaligned_le16(reg >> 2, &write_buf[SYSMON_I2C_REG_OFS]);
> + write_buf[SYSMON_I2C_INSTR_OFS] = SYSMON_I2C_INSTR_READ;
> +
> + ret = i2c_master_send(client, write_buf, sizeof(write_buf));
> + if (ret < 0)
> + return ret;
> + if (ret != sizeof(write_buf))
> + return -EIO;
> +
> + ret = i2c_master_recv(client, read_buf, sizeof(read_buf));
[Severity: High]
Does this sequence release the I2C bus lock between the command write and the
response read?
Using i2c_master_send() followed by i2c_master_recv() will independently
acquire and release the I2C adapter lock, generating a STOP condition on the
bus between the two operations.
If other devices share this I2C bus, could an interleaved transaction reset
the SysMon's internal state machine and cause the subsequent read to return
corrupted data?
Should this be implemented using a single i2c_transfer() with two messages
(using a Repeated START), or explicitly wrapped in i2c_lock_bus() and
i2c_unlock_bus() if a STOP condition is strictly required by the hardware?
> + if (ret < 0)
> + return ret;
> + if (ret != sizeof(read_buf))
> + return -EIO;
> +
> + *val = get_unaligned_le32(read_buf);
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617180147.3370346-1-salih.erim@amd.com?part=3
^ permalink raw reply
* Re: [PATCH v3 4/5] clk: renesas: rzv2h-cpg: Extract PLL calculation math into a library
From: Lad, Prabhakar @ 2026-06-17 20:45 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
linux-renesas-soc, linux-clk, devicetree, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CAMuHMdVN7dfF6bah0Ca=G-uHZwJ7JDyQb5NoE59PPfAuXp+mHg@mail.gmail.com>
Hi Geert,
Thank you for the review.
On Wed, Jun 17, 2026 at 11:05 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, 15 Jun 2026 at 12:48, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Move the common PLL and divider parameter calculation logic from the
> > core rzv2h-cpg driver into a standalone library file.
> >
> > Introduce the CLK_RZV2H_CPG_LIB Kconfig configuration symbol and create
> > rzv2h-cpg-lib.c to house rzv2h_cpg_get_pll_pars() and
> > rzv2h_cpg_get_pll_divs_pars().
> >
> > Keep rzv2h_get_pll_pars() and rzv2h_get_pll_divs_pars() in the original
> > driver as wrappers that call into the new library helper endpoints.
> > These wrappers are maintained for this cycle because they are actively
> > referenced by the DSI driver; they will be safely removed in a subsequent
> > cycle once the DSI driver is updated to use the new APIs from the library,
> > preventing cross-subsystem build breakages.
> >
> > This restructuring allows other Renesas SoC clock drivers, such as the
> > upcoming RZ/T2H and RZ/N2H platforms that utilize similar LCDC clock
> > divider mathematical logic, to share the iterative calculation helper
> > infrastructure without duplication.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/include/linux/clk/renesas.h
> > +++ b/include/linux/clk/renesas.h
> > @@ -213,4 +213,27 @@ static inline bool rzv2h_get_pll_divs_pars(const struct rzv2h_pll_limits *limits
> > }
> > #endif
> >
> > +#ifdef CONFIG_CLK_RZV2H_CPG_LIB
> > +bool rzv2h_cpg_get_pll_pars(const struct rzv2h_pll_limits *limits,
> > + struct rzv2h_pll_pars *pars, u64 freq_millihz);
> > +
> > +bool rzv2h_cpg_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
> > + struct rzv2h_pll_div_pars *pars,
> > + const u8 *table, u8 table_size, u64 freq_millihz);
> > +#else
> > +static inline bool rzv2h_cpg_get_pll_pars(const struct rzv2h_pll_limits *limits,
> > + struct rzv2h_pll_pars *pars,
> > + u64 freq_millihz)
> > +{
> > + return false;
> > +}
> > +
> > +static inline bool rzv2h_cpg_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
> > + struct rzv2h_pll_div_pars *pars,
> > + const u8 *table, u8 table_size,
> > + u64 freq_millihz)
> > +{
> > + return false;
> > +}
> > +#endif
> > #endif
>
> What about just dropping the old functions, and adding two simple
> compatibility defines in the header file:
>
> #define rzv2h_get_pll_pars rzv2h_cpg_get_pll_pars
> #define rzv2h_get_pll_divs_pars rzv2h_cpg_get_pll_divs_pars
>
Thats neat!
> That way there is less code to change in the next phase.
>
Agreed.
Cheers,
Prabhakar
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply
* Re: [PATCH v4 3/3] regulator: qcom-refgen: add support for the IPQ9650 SoC
From: sashiko-bot @ 2026-06-17 20:44 UTC (permalink / raw)
To: Kathiravan Thirumoorthy; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260617-ipq9650_refgen-v4-3-c505ea6c6661@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Unclocked MMIO read in get_status callback causes system crash.
- [Medium] Silent failure on missing clocks due to inadequate DT validation.
--
commit 90356084cf49412e26be88f69da7b99d50c27462
Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
regulator: qcom-refgen: add support for the IPQ9650 SoC
This commit adds support for the IPQ9650 SoC to the qcom-refgen regulator
driver. It introduces explicit clock handling for this block and a get_status
callback to report the regulator status.
> diff --git a/drivers/regulator/qcom-refgen-regulator.c b/drivers/regulator/qcom-refgen-regulator.c
> index 6a3795469927e..cc72e5a43ba62 100644
> --- a/drivers/regulator/qcom-refgen-regulator.c
> +++ b/drivers/regulator/qcom-refgen-regulator.c
[ ... ]
> +static int qcom_ipq9650_refgen_disable(struct regulator_dev *rdev)
> +{
> + struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
> +
> + clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
> +
> + drvdata->is_enabled = false;
[Severity: Critical]
Does this disable the clocks before updating the internal state?
If an is_enabled check were added to get_status() to prevent unclocked reads,
could a concurrent call to get_status() still crash the system if it
interleaves after clk_bulk_disable_unprepare() but before is_enabled is
set to false?
> +
> + return 0;
> +}
[ ... ]
> +static int qcom_ipq9650_refgen_get_status(struct regulator_dev *rdev)
> +{
> + u32 val;
> +
> + regmap_read(rdev->regmap, REFGEN_REG_REFGEN_STATUS, &val);
[Severity: Critical]
Can this unclocked MMIO read cause a system crash?
The get_status() callback reads the REFGEN_REG_REFGEN_STATUS register without
ensuring the regulator's clocks are enabled. Since user space can trigger a
read of /sys/class/regulator/regulator.X/status at any time, which invokes
get_status() locklessly, a disabled regulator could cause this regmap_read()
to trigger a synchronous external abort and hang the bus.
> + if (FIELD_GET(REFGEN_STATUS_OUT_MASK, val))
> + return REGULATOR_STATUS_ON;
> +
> + return REGULATOR_STATUS_OFF;
> +}
[ ... ]
> @@ -106,10 +192,23 @@ static int qcom_refgen_probe(struct platform_device *pdev)
[ ... ]
> + if (data->has_clocks) {
> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->num_clks = devm_clk_bulk_get_all(dev, &drvdata->clks);
> + if (drvdata->num_clks < 0)
[Severity: Medium]
Will this silently fail if the clocks are missing from the device tree?
If the clocks property is omitted, devm_clk_bulk_get_all() returns 0.
Since this code only checks for drvdata->num_clks < 0, it treats the missing
clocks as a success, and subsequent calls to clk_bulk_prepare_enable(0, ...)
will silently succeed without enabling any clocks.
Should this explicitly verify drvdata->num_clks > 0 since clocks are mandatory
for the IPQ9650 PHY to function properly?
> + return dev_err_probe(dev, drvdata->num_clks,
> + "failed to get clocks\n");
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-ipq9650_refgen-v4-0-c505ea6c6661@oss.qualcomm.com?part=3
^ permalink raw reply
* Re: [PATCH v7 2/4] PCI: Use standard wait times for PCIe link monitoring
From: sashiko-bot @ 2026-06-17 20:40 UTC (permalink / raw)
To: Thierry Reding; +Cc: robh, conor+dt, devicetree, linux-pci
In-Reply-To: <20260617-tegra264-pcie-v7-2-eae7ae964629@nvidia.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing cleanup of chained IRQ handlers and IRQ domains on error paths in multiple PCIe drivers leads to Use-After-Free.
- [High] advk_pcie_wait_for_link() fails to guarantee the mandatory 100ms post-reset delay before configuration requests.
--
commit 1edbe267835d4abccd02c4abc064267f402a8ce2
Author: Thierry Reding <treding@nvidia.com>
PCI: Use standard wait times for PCIe link monitoring
Instead of defining the wait values for each driver, use common values
defined in the core pci.h header file. Convert all drivers to msleep()
and use the existing definition.
> diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.c b/drivers/pci/controller/mobiveil/pcie-mobiveil.c
> index 62ecbaeb0a604..e8346851c49b6 100644
> --- a/drivers/pci/controller/mobiveil/pcie-mobiveil.c
> +++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.c
> @@ -218,11 +218,11 @@ int mobiveil_bringup_link(struct mobiveil_pcie *pcie)
> int retries;
>
> /* check if the link is up or not */
> - for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> + for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
> if (mobiveil_pcie_link_up(pcie))
> return 0;
>
> - usleep_range(LINK_WAIT_MIN, LINK_WAIT_MAX);
> + msleep(PCIE_LINK_WAIT_SLEEP_MS);
> }
>
> dev_err(&pcie->pdev->dev, "link never came up\n");
[Severity: High]
This isn't a bug introduced by this patch, but is there a potential
Use-After-Free in the error paths handling these link bring-up failures?
In the caller mobiveil_pcie_host_probe(), if mobiveil_bringup_link() fails, it
returns directly:
ret = mobiveil_bringup_link(pcie);
if (ret) {
dev_info(dev, "link bring-up failed\n");
return ret;
}
Since chained handlers are set up earlier using
irq_set_chained_handler_and_data(..., pcie), returning an error causes the
devres-managed pcie struct to be freed. If the interrupt fires after this, won't
the active handler dereference the freed pcie pointer?
A similar issue appears in nwl_pcie_probe() if nwl_wait_for_link() fails. Should
these error paths unregister the chained handlers and call irq_domain_remove()
to prevent this UAF and resource leak?
[ ... ]
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index fd9c7d53e8a78..272c5c8fc1e5c 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
[ ... ]
> @@ -350,13 +347,13 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
> int retries;
>
> /* check if the link is up or not */
> - for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> + for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
> if (advk_pcie_link_up(pcie)) {
> pci_host_common_link_train_delay(pcie->link_gen);
> return 0;
> }
>
> - usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
> + msleep(PCIE_LINK_WAIT_SLEEP_MS);
> }
[Severity: High]
This is a pre-existing issue, but does advk_pcie_wait_for_link() fail to
guarantee the mandatory 100ms post-reset delay before configuration requests?
The PCIe Base Specification mandates a minimum 100ms delay after a conventional
reset (e.g., PERST# deassertion) before the host can send Configuration
Requests. The caller advk_pcie_setup_hw() asserts PERST# and delegates the
post-reset delay to this function.
If the link trains rapidly and reaches L0 early, this loop returns immediately.
For link speeds of Gen2 or lower, pci_host_common_link_train_delay() is a no-op,
so no additional delay is added.
Does this allow the driver to issue Configuration Requests before the endpoint's
100ms initialization period has elapsed, potentially causing enumeration failures
for compliant endpoints?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-tegra264-pcie-v7-0-eae7ae964629@nvidia.com?part=2
^ permalink raw reply
* Re: [PATCH v4 03/10] dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED reg range
From: sashiko-bot @ 2026-06-17 20:31 UTC (permalink / raw)
To: Jun Yan; +Cc: devicetree, linux-input, robh, dmitry.torokhov, conor+dt
In-Reply-To: <20260617150318.753148-4-jerrysteve1101@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete Refactoring / Code and Commit Message Mismatch
--
commit e8499e0ca62db6a2ee6dc0986e7c04ea1337f6c7
Author: Jun Yan <jerrysteve1101@gmail.com>
Add datasheet links for all supported CAP11xx variants.
Update LED node regex and replace enum constraints with minimum/maximum
for LED reg ranges in preparation for CAP1114 support. Drop unnecessary
led unit-address pattern.
> diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> index 7ade03f1b32b..eabf06a1163e 100644
> --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
[ ... ]
> @@ -131,7 +140,9 @@ patternProperties:
>
> properties:
> reg:
> - enum: [0, 1, 2, 3, 4, 5, 6, 7]
> + description: LED channel number
> + minimum: 0
> + maximum: 7
[Severity: Medium]
Does this fully prepare the schema for CAP1114 support as mentioned in the
commit message?
While the constraints on the reg property were updated here, the main
patternProperties definition for the LED node just above this still uses
"^led@[0-7]$".
Since CAP1114 has 11 LED channels, wouldn't the schema fail validation for
nodes like led@8 and led@9 because the top-level pattern wasn't also updated?
[ ... ]
> @@ -158,7 +169,7 @@ allOf:
> - microchip,cap1298
> then:
> patternProperties:
> - "^led@[0-7]$": false
> + "^led@": false
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617150318.753148-1-jerrysteve1101@gmail.com?part=3
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox