* Re: [RFC PATCH 1/3] dt-bindings: pinctrl: mt8516/mt8167: Move compatibles from mt66xx to mt6795
From: Luca Leonardo Scorcia @ 2026-06-25 16:47 UTC (permalink / raw)
To: Conor Dooley
Cc: linux-mediatek, Sean Wang, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20260625-cameo-siamese-cd78c349519c@spud>
Hi,
> I've not done a very through analysis, but this seems like a massive ABI
> break.
> The change you're trying to make here will mean that new kernels will
> not work with older devicetrees AFAICT.
Correct, that's the reason I sent it as an RFC (I mentioned this in
the cover letter). I am new to kernel work and I'm not sure how to
deal with this change. On one hand I am almost certain now that the
upstream driver has never been used in actual devices, since the older
code was only partially merged and also, as Sashiko correctly pointed
out in [1], it had serious errors when matched against the data sheet:
Sashiko:
> Does this configuration cause a regression in pin multiplexing across the SoC?
> The legacy driver used a 4-bit shift per pin to pack 5 pins per 32-bit
> register. By passing 3 as the width here, the framework calculates mode
> offsets using 3 bits per pin. This causes pinmux writes to align with
> the wrong bits and can overwrite the configurations of adjacent pins.
Data sheet here clearly shows 3 bits per pin are used to choose the
pin function.
On the other hand I know that breaking the ABI is a big no. But what
would be an appropriate solution? Maybe duplicating the driver with a
different name, something like mediatek,mt8167-pinctrl-v2? Is there
another driver I could have a look at to learn how to approach this
problem?
Sashiko also pointed out some other minor issues with the register
maps I already fixed locally after confirming with the data sheet, but
did not provide clues about how to solve the ABI breakage.
[1] https://sashiko.dev/#/message/20260625111629.6CD701F000E9%40smtp.kernel.org
Thank you for your time!
--
Luca Leonardo Scorcia
l.scorcia@gmail.com
^ permalink raw reply
* Re: [PATCH v2] spi: dt-bindings: microchip,pic32mzda-sqi: Convert to DT schema
From: Conor Dooley @ 2026-06-25 16:34 UTC (permalink / raw)
To: Udaya Kiran Challa
Cc: tsbogend, robh, krzk+dt, conor+dt, skhan, me, linux-spi,
devicetree, linux-kernel
In-Reply-To: <20260625060903.14661-1-challauday369@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 78 bytes --]
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor
From: Conor Dooley @ 2026-06-25 16:34 UTC (permalink / raw)
To: hehuan1
Cc: linux, robh, krzk+dt, conor+dt, p.zabel, linux-hwmon, devicetree,
linux-kernel, ningyu, linmin, pinkesh.vaghela, luyulin,
dongxuyang
In-Reply-To: <20260625061147.1631-1-hehuan1@eswincomputing.com>
[-- Attachment #1: Type: text/plain, Size: 78 bytes --]
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/7] dt-bindings: display: verisilicon,dc: generalize for single-output variants
From: Conor Dooley @ 2026-06-25 16:33 UTC (permalink / raw)
To: Joey Lu
Cc: zhengxingda, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt, ychuang3, schung, yclu4,
dri-devel, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20260625094449.708386-2-a0987203069@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3667 bytes --]
On Thu, Jun 25, 2026 at 05:44:43PM +0800, Joey Lu wrote:
> The verisilicon,dc binding was originally written for the T-Head TH1520
> SoC carrying a DC8200, and hard-codes five clocks, three resets and two
> output ports.
>
> Add the Nuvoton MA35D1 DCUltraLite (nuvoton,ma35d1-dcu) to the binding.
> The DCUltraLite uses only two clocks (core, pix0) and one reset (core),
> with a single output port.
>
> Use allOf/if blocks to express per-variant constraints rather than
> hard-coding the DC8200 topology at the top level. Each compatible's
> block constrains the clock and reset item counts; the nuvoton block
> additionally overrides clock-names to the two names it actually uses.
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
> .../bindings/display/verisilicon,dc.yaml | 57 +++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> index 9dc35ab973f2..1e751f3c7ce8 100644
> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> @@ -17,6 +17,7 @@ properties:
> items:
> - enum:
> - thead,th1520-dc8200
> + - nuvoton,ma35d1-dcu
> - const: verisilicon,dc # DC IPs have discoverable ID/revision registers
>
> reg:
> @@ -77,6 +78,62 @@ required:
> - clock-names
> - ports
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: thead,th1520-dc8200
> + then:
> + properties:
> + clocks:
> + minItems: 5
> + maxItems: 5
> +
> + clock-names:
> + minItems: 5
> + maxItems: 5
All the maxItems here repeat the maximum constraint and do nothing.
Since you didn't change the minimum constraint at the top level, your
minItems also do nothing.
> +
> + resets:
> + minItems: 3
> + maxItems: 3
> +
> + reset-names:
> + minItems: 3
> + maxItems: 3
> +
> + required:
> + - resets
> + - reset-names
Both conditional sections have this, but the original binding doesn't
require these for the thead device. This is a functional change
therefore and shouldn't be in a patch calling itself "generalise for
single ended variants".
FWIW, adding your new compatible shouldn't really be in a patch with
that subject either, it really should say "add support for nuvoton
ma35d1" or something.
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: nuvoton,ma35d1-dcu
> + then:
> + properties:
> + clocks:
> + minItems: 2
Anything that updates the minimum constraint should be done at the top
level of this schema. The conditional section should then tighten the
constraint, in this case that means only having maxItems.
> + maxItems: 2
> +
> + clock-names:
> + items:
> + - const: core
> + - const: pix0
Does this even work when the top level schema thinks clock 2 should be
called axi?
> +
> + resets:
> + minItems: 1
> + maxItems: 1
> +
> + reset-names:
> + items:
> + - const: core
This is just maxItems: 1.
pw-bot: changes-requested
Thanks,
Conor.
> +
> + required:
> + - resets
> + - reset-names
> +
> additionalProperties: false
>
> examples:
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 1/3] dt-bindings: pinctrl: mt8516/mt8167: Move compatibles from mt66xx to mt6795
From: Conor Dooley @ 2026-06-25 16:28 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Sean Wang, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20260625104742.113803-2-l.scorcia@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2242 bytes --]
On Thu, Jun 25, 2026 at 12:46:30PM +0200, Luca Leonardo Scorcia wrote:
> Pinctrl settings for MediaTek mt8516-mt8167 SoCs use two reg base
> addresses, one for GPIO and the other for EINT, as it is common in the
> "Paris" pinctrl platform that is described in the MediaTek mt6795 docs.
>
> Move the binding compatible for these two SoCs from mt66xx to the mt6796
> one as a prerequisite for migrating the pinctrl driver to the
> pinctrl-paris platform.
I've not done a very through analysis, but this seems like a massive ABI
break.
The change you're trying to make here will mean that new kernels will
not work with older devicetrees AFAICT.
>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> .../devicetree/bindings/pinctrl/mediatek,mt65xx-pinctrl.yaml | 2 --
> .../devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml | 5 ++++-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt65xx-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt65xx-pinctrl.yaml
> index 1468c6f87cfa..0cff2a352b1f 100644
> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt65xx-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt65xx-pinctrl.yaml
> @@ -22,9 +22,7 @@ properties:
> - mediatek,mt7623-pinctrl
> - mediatek,mt8127-pinctrl
> - mediatek,mt8135-pinctrl
> - - mediatek,mt8167-pinctrl
> - mediatek,mt8173-pinctrl
> - - mediatek,mt8516-pinctrl
>
> reg:
> maxItems: 1
> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml
> index 9a937f414cc9..c703de72e1d5 100644
> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml
> @@ -15,7 +15,10 @@ description:
>
> properties:
> compatible:
> - const: mediatek,mt6795-pinctrl
> + enum:
> + - mediatek,mt6795-pinctrl
> + - mediatek,mt8167-pinctrl
> + - mediatek,mt8516-pinctrl
>
> gpio-controller: true
>
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
From: Conor Dooley @ 2026-06-25 16:24 UTC (permalink / raw)
To: Chi-Wen Weng
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-arm-kernel, linux-iio, devicetree, linux-kernel, cwweng
In-Reply-To: <20260625110638.38438-2-cwweng.linux@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 540 bytes --]
On Thu, Jun 25, 2026 at 07:06:37PM +0800, Chi-Wen Weng wrote:
> From: Chi-Wen Weng <cwweng@nuvoton.com>
>
> Add devicetree binding for the Enhanced ADC controller found on
> Nuvoton MA35D1 SoCs.
>
> The controller has one register region, one interrupt and one functional
> clock. ADC inputs are described using standard channel child nodes,
> including optional differential channel pairs.
>
> Signed-off-by: Chi-Wen Weng <cwweng@nuvoton.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: ti: Add bindings for PHYTEC AM67x based hardware
From: Conor Dooley @ 2026-06-25 16:22 UTC (permalink / raw)
To: Nathan Morrisson
Cc: nm, vigneshr, kristo, robh, krzk+dt, conor+dt, linux-arm-kernel,
devicetree, linux-kernel, upstream
In-Reply-To: <20260625160214.4001298-1-nmorrisson@phytec.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] dt-bindings: dma: xlnx,axi-dma: Restore xlnx,flush-fsync as u32
From: Conor Dooley @ 2026-06-25 16:21 UTC (permalink / raw)
To: Suraj Gupta
Cc: vkoul, Frank.Li, robh, krzk+dt, conor+dt, michal.simek,
radhey.shyam.pandey, dmaengine, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260625161016.1249570-1-suraj.gupta2@amd.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: hwmon: chipcap2: Add label property
From: sashiko-bot @ 2026-06-25 16:21 UTC (permalink / raw)
To: Flaviu Nistor; +Cc: linux-hwmon, robh, conor+dt, devicetree
In-Reply-To: <20260625160423.17882-1-flaviu.nistor@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The threaded interrupt handlers modify alarm state flags without locking, creating a race condition with sysfs reads that can result in lost alarm events.
--
commit 7c977021bab8a6a8bb249ac3b4ad711dbe88c4ff
Author: Flaviu Nistor <flaviu.nistor@gmail.com>
dt-bindings: hwmon: chipcap2: Add label property
This commit adds support for an optional label property similar to other
hwmon devices. This allows assigning distinct names to each CHIPCAP2
sensor instance on boards with multiple sensors.
> diff --git a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> index 17351fdbefce6..56b0cecfca5fb 100644
> --- a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> @@ -45,6 +45,8 @@ properties:
> - const: low
> - const: high
>
> + label: true
> +
> vdd-supply:
[ ... ]
While reviewing the hwmon driver code for this hardware, I noticed a
potential locking issue in the interrupt handling.
> --- a/drivers/hwmon/chipcap2.c
> +++ b/drivers/hwmon/chipcap2.c
[ ... ]
> static irqreturn_t cc2_low_interrupt(int irq, void *data)
> {
> struct cc2_data *cc2 = data;
>
> if (cc2->process_irqs) {
> hwmon_notify_event(cc2->hwmon, hwmon_humidity,
> hwmon_humidity_min_alarm, CC2_CHAN_HUMIDITY);
> cc2->rh_alarm.low_alarm = true;
^^^^
[Severity: High]
This isn't a bug introduced by this patch, but the threaded interrupt
handler modifies the min alarm flag without acquiring a lock. Could this
create a race condition where an active alarm gets overwritten?
> }
>
> return IRQ_HANDLED;
> }
>
> static irqreturn_t cc2_high_interrupt(int irq, void *data)
> {
> struct cc2_data *cc2 = data;
>
> if (cc2->process_irqs) {
> hwmon_notify_event(cc2->hwmon, hwmon_humidity,
> hwmon_humidity_max_alarm, CC2_CHAN_HUMIDITY);
> cc2->rh_alarm.high_alarm = true;
^^^^
[Severity: High]
This is a pre-existing issue, but the max alarm flag is also modified
here without locking.
> }
>
> return IRQ_HANDLED;
> }
>
> static int cc2_humidity_min_alarm_status(struct cc2_data *data, long *val)
> {
> long measurement, min_hyst;
> int ret;
>
> ret = cc2_read_hyst_and_measure(data, CC2_R_ALARM_L_OFF, &min_hyst,
> &measurement);
> if (ret < 0)
> return ret;
>
> if (data->rh_alarm.low_alarm) {
> *val = (measurement < min_hyst) ? 1 : 0;
> data->rh_alarm.low_alarm = *val;
^^^^
[Severity: High]
This isn't a bug introduced by this patch, but the sysfs read thread
performs an unprotected read-modify-write on the alarm state flag here.
If a sysfs read of the alarm status evaluates the current measurement
over the I2C bus, and cc2_low_interrupt() concurrently receives an
alarm setting low_alarm to true, could the sysfs thread subsequently
overwrite the active alarm flag with 0 based on stale measurement data?
The hwmon subsystem documentation mentions that hwmon_lock() or custom
locking should be used to serialize sysfs operations with interrupt
handling.
> } else {
> *val = 0;
> }
>
> return 0;
> }
>
> static int cc2_humidity_max_alarm_status(struct cc2_data *data, long *val)
> {
> long measurement, max_hyst;
> int ret;
>
> ret = cc2_read_hyst_and_measure(data, CC2_R_ALARM_H_OFF, &max_hyst,
> &measurement);
> if (ret < 0)
> return ret;
>
> if (data->rh_alarm.high_alarm) {
> *val = (measurement > max_hyst) ? 1 : 0;
> data->rh_alarm.high_alarm = *val;
^^^^
[Severity: High]
This is a pre-existing issue, but the max alarm sysfs read also writes
back the max alarm flag without locking. Does this present the same risk
of lost alarm events if cc2_high_interrupt() runs concurrently?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625160423.17882-1-flaviu.nistor@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH net] net: ethernet: qualcomm: ppe: Demote from supported and fix maintainer addresses
From: patchwork-bot+netdevbpf @ 2026-06-25 16:20 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: andersson, mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt,
jie.luo, andrew+netdev, davem, edumazet, kuba, pabeni,
quic_leiwei, quic_suruchia, quic_pavir, linux-kernel,
linux-arm-msm, linux-clk, devicetree, netdev
In-Reply-To: <20260623073307.36483-2-krzysztof.kozlowski@oss.qualcomm.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 23 Jun 2026 09:33:08 +0200 you wrote:
> Emails to the maintainer of Qualcomm PPE Ethernet driver (Luo Jie
> <quic_luoj@quicinc.com>) bounce permanently (full mailbox), because the
> "quicinc.com" addresses were deprecated for public work. All Qualcomm
> contributors are aware of that and were asked to fix their addresses.
>
> Driver is not supported - in terms of how netdev understands supported
> commitment - if maintainer does not care to receive the patches for its
> code, so demote it to "maintained" to reflect true status.
>
> [...]
Here is the summary with links:
- [net] net: ethernet: qualcomm: ppe: Demote from supported and fix maintainer addresses
https://git.kernel.org/netdev/net/c/efd7fb21bad8
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net] dt-bindings: net: renesas,ether: Drop example "ethernet-phy-ieee802.3-c22" fallback
From: patchwork-bot+netdevbpf @ 2026-06-25 16:20 UTC (permalink / raw)
To: Rob Herring
Cc: niklas.soderlund, andrew+netdev, davem, edumazet, kuba, pabeni,
krzk+dt, conor+dt, geert+renesas, magnus.damm, sergei.shtylyov,
netdev, linux-renesas-soc, devicetree, linux-kernel
In-Reply-To: <20260624150250.131966-2-robh@kernel.org>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 24 Jun 2026 10:02:50 -0500 you wrote:
> Fix the Micrel PHY in the example which shouldn't have the
> fallback "ethernet-phy-ieee802.3-c22" compatible:
>
> Documentation/devicetree/bindings/net/renesas,ether.example.dtb: ethernet-phy@1 \
> (ethernet-phy-id0022.1537): compatible: ['ethernet-phy-id0022.1537', 'ethernet-phy-ieee802.3-c22'] is too long
> from schema $id: http://devicetree.org/schemas/net/micrel.yaml
>
> [...]
Here is the summary with links:
- [net] dt-bindings: net: renesas,ether: Drop example "ethernet-phy-ieee802.3-c22" fallback
https://git.kernel.org/netdev/net/c/14eb1d2c03b3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2 7/8] dt-bindings: riscv: Add generic CBQRI controller binding
From: Conor Dooley @ 2026-06-25 16:19 UTC (permalink / raw)
To: Drew Fustini
Cc: Adrien Ricciardi, Alexandre Ghiti, Atish Kumar Patra, Atish Patra,
Babu Moger, Ben Horgan, Borislav Petkov, Chen Pei, Conor Dooley,
Conor Dooley, Dave Hansen, Dave Martin, Fenghua Yu, Gong Shuai,
Gong Shuai, guo.wenjia23, James Morse, Kornel Dulęba,
Krzysztof Kozlowski, liu.qingtao2, Liu Zhiwei, Palmer Dabbelt,
Paul Walmsley, Peter Newman, Radim Krčmář,
Reinette Chatre, Rob Herring, Samuel Holland,
Sebastian Andrzej Siewior, Tony Luck, Vasudevan Srinivasan,
Ved Shanbhogue, Weiwei Li, yunhui cui, linux-kernel, linux-riscv,
x86, devicetree, linux-rt-devel, linux-doc
In-Reply-To: <20260624-dfustini-atl-sc-cbqri-dt-v2-7-2f8049fd902b@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 5686 bytes --]
On Wed, Jun 24, 2026 at 06:38:35PM -0700, Drew Fustini wrote:
> Document the generic compatibles for capacity and bandwidth controllers
> that implement the RISC-V CBQRI specification. The binding also
> describes the common riscv,cbqri-rcid and riscv,cbqri-mcid properties,
> and the optional riscv,cbqri-cache phandle that links a capacity
> controller to the cache whose capacity it allocates.
>
> Assisted-by: Claude:claude-opus-4-8
> Co-developed-by: Adrien Ricciardi <aricciardi@baylibre.com>
> Signed-off-by: Adrien Ricciardi <aricciardi@baylibre.com>
> Signed-off-by: Drew Fustini <fustini@kernel.org>
> ---
> .../devicetree/bindings/riscv/riscv,cbqri.yaml | 97 ++++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 98 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/riscv/riscv,cbqri.yaml b/Documentation/devicetree/bindings/riscv/riscv,cbqri.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..5d6be645381780e187b39e60c3bb487fdf2cfb69
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/riscv/riscv,cbqri.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/riscv/riscv,cbqri.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RISC-V Capacity and Bandwidth QoS Register Interface (CBQRI) controller
> +
> +description: |
> + The RISC-V CBQRI specification defines capacity-controller and
> + bandwidth-controller register blocks that allocate cache capacity and memory
> + bandwidth to resource-control IDs (RCIDs) and monitor usage per
> + monitoring-counter ID (MCID):
> + https://github.com/riscv-non-isa/riscv-cbqri/blob/main/riscv-cbqri.pdf
> +
> + Allocation and monitoring share one register block, and a controller may
> + implement either or both. A driver discovers which at runtime from the
> + capabilities register, so the compatible names only the controller type. It
> + does not distinguish allocation-only, monitoring-only or combined
> + controllers, and no property declares monitoring support.
> +
> +maintainers:
> + - Drew Fustini <fustini@kernel.org>
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - description: Tenstorrent Ascalon Shared Cache
> + const: tenstorrent,ascalon-sc-cbqri
> + - const: riscv,cbqri-capacity-controller
> + - enum:
> + - riscv,cbqri-capacity-controller
> + - riscv,cbqri-bandwidth-controller
Please modify this, as has been done for other riscv spec related
bindings, to let people get away without using device-specific
compatibles.
In this case, you can just delete the first entry from this enum, since
it already has a user and only have to implement this feedback for the
second entry.
pw-bot: changes-requested
> +
> + reg:
> + maxItems: 1
> + description:
> + The CBQRI controller register block.
> +
> + riscv,cbqri-rcid:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + The maximum number of RCIDs the controller supports. RCIDs are the
> + resource-control IDs that allocation operations target.
> +
> + riscv,cbqri-mcid:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + The maximum number of MCIDs the controller supports. MCIDs are the
> + monitoring-counter IDs that usage-monitoring operations target. Present
> + on controllers that implement monitoring.
> +
> + riscv,cbqri-cache:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the cache node whose capacity this controller allocates.
> + Applies to capacity controllers that back a CPU cache. The cache level
> + and the harts sharing it are taken from that node's cache topology.
Architecturally, is it impossible for a capacity controller to control
more than one cache?
> +
> +required:
> + - compatible
> + - reg
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: tenstorrent,ascalon-sc-cbqri
> + then:
> + required:
> + - riscv,cbqri-rcid
> + - riscv,cbqri-cache
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + l2_cache: l2-cache {
> + compatible = "cache";
> + cache-level = <2>;
> + cache-unified;
> + cache-size = <0xc00000>;
> + cache-sets = <512>;
> + cache-block-size = <64>;
> + };
> +
> + cache-controller@a21a00c0 {
> + compatible = "tenstorrent,ascalon-sc-cbqri",
> + "riscv,cbqri-capacity-controller";
Is this or is this not a cache controller?
The compatible and fact that the property points to an actual cache
controller suggests that this is not.
Cheers,
Conor.
> + reg = <0xa21a00c0 0xf40>;
> + riscv,cbqri-rcid = <16>;
> + riscv,cbqri-cache = <&l2_cache>;
> + };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e1092165046c773771b055869030bc1bdb64b16..64a95a4d795a57033d3f36200d98cfb4a013ab94 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23298,6 +23298,7 @@ M: Drew Fustini <fustini@kernel.org>
> R: yunhui cui <cuiyunhui@bytedance.com>
> L: linux-riscv@lists.infradead.org
> S: Supported
> +F: Documentation/devicetree/bindings/riscv/riscv,cbqri.yaml
> F: arch/riscv/include/asm/qos.h
> F: arch/riscv/include/asm/resctrl.h
> F: arch/riscv/kernel/qos.c
>
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: ti: Add support for the phyCORE-AM67x
From: sashiko-bot @ 2026-06-25 16:17 UTC (permalink / raw)
To: Nathan Morrisson; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260625160214.4001298-2-nmorrisson@phytec.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The PCF8574 GPIO expanders specify `IRQ_TYPE_LEVEL_LOW`, which is unsupported by the K3 GPIO controller and will cause probe failure.
- [Medium] The `reset-gpios` for `pcie0_rc` is specified as `GPIO_ACTIVE_HIGH`, which inverses the standard PCIe PERST# active-low logic.
- [Medium] Mismatch between `mcasp1` serializer configuration and its pin multiplexing: AXR2 is configured for RX but is not pinned out.
- [Medium] Button 'key-menu' connected to PCF8574 uses GPIO_ACTIVE_HIGH, causing inverted logic.
- [Low] Typo in the micbias voltage property for the `tlv320aic3110` audio codec.
--
commit 736b51d7a471b4b2aa4f1281a1e87be6e988051c
Author: Nathan Morrisson <nmorrisson@phytec.com>
arm64: dts: ti: Add support for the phyCORE-AM67x
Add support for the PHYTEC phyCORE-AM67x SoM and the corresponding
phyBOARD-Rigel carrier board. The phyCORE-AM67x SoM uses the TI AM67x SoC
and can come with different sizes and models of DDR, eMMC, and SPI NOR
Flash.
Link: https://www.phytec.com/product/phycore-am67x/
Link: https://www.phytec.com/product/phyboard-am67x-development-kit/
> diff --git a/arch/arm64/boot/dts/ti/k3-am6754-phyboard-rigel.dts b/arch/arm64/boot/dts/ti/k3-am6754-phyboard-rigel.dts
> new file mode 100644
> index 0000000000000..7853d4f5d3b95
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am6754-phyboard-rigel.dts
[ ... ]
> + keys {
> + compatible = "gpio-keys";
> + autorepeat;
> + pinctrl-names = "default";
> + pinctrl-0 = <&gpio_keys_pins_default>;
[ ... ]
> + key-menu {
> + label = "menu";
> + linux,code = <KEY_MENU>;
> + gpios = <&gpio_exp1 4 GPIO_ACTIVE_HIGH>;
[Severity: Medium]
Does this configuration cause the button to register as permanently pressed?
The key-menu button is connected to gpio_exp1, which is a PCF8574 expander.
These pins have internal weak pull-ups, meaning their default resting state
is HIGH, and buttons typically pull them to GND (LOW). Specifying
GPIO_ACTIVE_HIGH inverses the logical state unless a strong external
pull-down resistor is present.
> + };
> + };
[ ... ]
> +&main_pmx0 {
[ ... ]
> + main_mcasp1_pins_default: main-mcasp1-default-pins {
> + pinctrl-single,pins = <
> + J722S_IOPAD(0x0090, PIN_INPUT, 2) /* (P27) GPMC0_BE0n_CLE.MCASP1_ACLKX */
> + J722S_IOPAD(0x0098, PIN_INPUT, 2) /* (V21) GPMC0_WAIT0.MCASP1_AFSX */
> + J722S_IOPAD(0x008c, PIN_OUTPUT, 2) /* (N23) GPMC0_WEn.MCASP1_AXR0 */
> + >;
> + };
[Severity: Medium]
Will audio capture on the mcasp1 interface fail here because the data pin
is not connected?
The mcasp1 node below configures serial-dir index 2 to RX, enabling
MCASP1_AXR2. However, the corresponding pin configuration for MCASP1_AXR2
is missing from main_mcasp1_pins_default. The pin remains in its default
state, meaning the controller receives no data.
[ ... ]
> +&main_i2c1 {
[ ... ]
> + audio_codec: audio-codec@18 {
> + compatible = "ti,tlv320aic3110";
> + reg = <0x18>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&audio_ext_refclk1_pins_default>;
> + #sound-dai-cells = <0>;
> + ai3xx-micbias-vg = <2>;
[Severity: Low]
Is there a typo in the micbias voltage property name?
The ti,tlv320aic3110 codec driver expects the property ai31xx-micbias-vg.
Using ai3xx-micbias-vg causes the driver to ignore this configuration and
fall back to the default 2.0V bias, which could cause incorrect microphone
gain or distortion.
> + reset-gpios = <&gpio_exp1 7 GPIO_ACTIVE_LOW>;
[ ... ]
> + gpio_exp0: gpio@20 {
> + compatible = "nxp,pcf8574";
> + reg = <0x20>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&gpio_exp0_int_pins_default>;
> + interrupt-parent = <&main_gpio0>;
> + interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
[Severity: High]
Will this expander fail to probe due to an unsupported interrupt type?
The K3 GPIO controller driver (gpio-davinci.c) strictly supports only
edge-triggered interrupts and returns -EINVAL for level types in its
irq_set_type callback. This would prevent the expanders and all attached
devices from probing.
> + gpio-line-names = "CSI3_STROBE", "CSI3_TRIGGER",
[ ... ]
> +&pcie0_rc {
> + pinctrl-names = "default";
> + pinctrl-0 = <&main_pcie_pins_default>;
> + num-lanes = <1>;
> + phys = <&serdes1_pcie_link>;
> + phy-names = "pcie-phy";
> + reset-gpios = <&main_gpio0 31 GPIO_ACTIVE_HIGH>;
[Severity: Medium]
Could this cause the PCIe endpoint to fail enumeration by keeping the
reset line asserted?
The standard PCIe PERST# signal is active-low. Specifying GPIO_ACTIVE_HIGH
causes the host controller to invert this logic, driving the line high when
it intends to assert reset, and low when it releases it. Unless the board
has a physical inverter on this line, the endpoint will remain in reset.
> + status = "okay";
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625160214.4001298-1-nmorrisson@phytec.com?part=2
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: ti: Add bindings for PHYTEC AM67x based hardware
From: sashiko-bot @ 2026-06-25 16:15 UTC (permalink / raw)
To: Nathan Morrisson; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260625160214.4001298-1-nmorrisson@phytec.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a tab character for indentation in a YAML file, which is forbidden by the YAML specification and breaks schema validation.
--
commit 969d143e093fd0e0c150cfa118a1a7839d600df9
Author: Nathan Morrisson <nmorrisson@phytec.com>
dt-bindings: arm: ti: Add bindings for PHYTEC AM67x based hardware
This commit adds device tree bindings for the AM67x based phyCORE-AM67x SoM
and phyBOARD-Rigel.
> diff --git a/Documentation/devicetree/bindings/arm/ti/k3.yaml b/Documentation/devicetree/bindings/arm/ti/k3.yaml
> index 2a6a9441c23de..9abe252fc7080 100644
> --- a/Documentation/devicetree/bindings/arm/ti/k3.yaml
> +++ b/Documentation/devicetree/bindings/arm/ti/k3.yaml
> @@ -215,6 +215,13 @@ properties:
> - ti,j722s-evm
> - const: ti,j722s
>
> + - description: K3 AM67 SoC PHYTEC phyBOARD-Rigel
> + items:
> + - enum:
> + - phytec,am6754-phyboard-rigel
[Severity: Low]
Does this line use a tab character for indentation? YAML syntax strictly
forbids the use of tabs for indentation, which will cause dt_binding_check
to fail with a parsing syntax error.
> + - const: phytec,am67-phycore-som
> + - const: ti,j722s
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625160214.4001298-1-nmorrisson@phytec.com?part=1
^ permalink raw reply
* Re: [PATCH v3 4/4] edac: xilinx: Add EDAC support for Versal XilSem
From: Pandey, Radhey Shyam @ 2026-06-25 16:13 UTC (permalink / raw)
To: Rama devi Veggalam, bp, tony.luck, michal.simek, robh, krzk+dt,
conor+dt
Cc: linux-kernel, linux-edac, devicetree, james.morse, mchehab, rric,
git
In-Reply-To: <20260624212545.2850787-5-rama.devi.veggalam@amd.com>
> Xilinx Versal Soft Error Mitigation (XilSEM) is responsible for reporting
> and optionally correcting soft errors in Configuration Memory of Versal.
> The Configuration Memory includes Configuration RAM and
> Network on Chip (NoC) peripheral interconnect (NPI) Registers.
>
> The Configuration RAM (CRAM) memory is used for storing configuration
> data for the programmable logic (PL) fabric. The NPI registers are used
> for configuring the memory controllers, miscellaneous integrated hardware,
> NoC interface units in the Veral device.
>
> Add support to handle correctable and uncorrectable error events
> from XilSEM.
>
> Add sysfs interface for XilSEM scan operations
> initialize, start, stop scan, error inject, read ECC, scan status and
> configuration values.
>
> Signed-off-by: Rama devi Veggalam <rama.devi.veggalam@amd.com>
> ---
<snip>
> static int mc_probe(struct platform_device *pdev)
> {
> - void __iomem *ddrmc_baseaddr, *ddrmc_noc_baseaddr;
> + void __iomem *ddrmc_baseaddr, *ddrmc_noc_baseaddr, *sem_baseaddr;
> struct edac_mc_layer layers[2];
> struct mem_ctl_info *mci;
> u8 num_chans, num_csrows;
> struct edac_priv *priv;
> u32 edac_mc_id, regval;
> + u32 family_code;
> int rc;
>
> ddrmc_baseaddr = devm_platform_ioremap_resource_byname(pdev, "base");
> @@ -1094,6 +2365,10 @@ static int mc_probe(struct platform_device *pdev)
> if (!get_ecc_state(ddrmc_baseaddr))
> return -ENXIO;
>
> + sem_baseaddr = devm_platform_ioremap_resource_byname(pdev, "semrtca");
> + if (IS_ERR(sem_baseaddr))
> + return PTR_ERR(sem_baseaddr);
> +
As mentioned by Krzysztof in DT review this change breaks existing
system. Please ioremap this semrtca resource only for xilSEM support.
We can make use of compatible based match data.
> /* Allocate ID number for the EMIF controller */
> edac_mc_id = emif_get_id(pdev->dev.of_node);
>
> @@ -1124,9 +2399,29 @@ static int mc_probe(struct platform_device *pdev)
> priv = mci->pvt_info;
> priv->ddrmc_baseaddr = ddrmc_baseaddr;
> priv->ddrmc_noc_baseaddr = ddrmc_noc_baseaddr;
> + priv->sem_baseaddr = sem_baseaddr;
> priv->ce_cnt = 0;
> priv->ue_cnt = 0;
> priv->mc_id = edac_mc_id;
> + priv->xsem_rtca = NULL;
> +
> + /* Allocate and initialize XilSem RTCA structure */
> + priv->xsem_rtca = devm_kzalloc(&pdev->dev,
> + sizeof(struct xsem_rtca_priv),
> + GFP_KERNEL);
> + if (!priv->xsem_rtca) {
> + edac_printk(KERN_ERR, EDAC_MC,
> + "Failed to allocate xsem_rtca\n");
> + rc = -ENOMEM;
> + goto free_edac_mc;
> + }
> +
> + priv->xsem_rtca->slr_info = devm_kzalloc(&pdev->dev, sizeof(struct xsem_ssit_status),
> + GFP_KERNEL);
> + if (!priv->xsem_rtca->slr_info) {
> + rc = -ENOMEM;
> + goto free_edac_mc;
> + }
>
> mc_init(mci, pdev);
>
> @@ -1147,6 +2442,41 @@ static int mc_probe(struct platform_device *pdev)
> goto del_mc;
> }
>
> + /* Create XilSem sysfs attributes only if XilSem is available */
> + rc = xsem_edac_create_sysfs_attributes(mci);
> + if (rc) {
> + edac_printk(KERN_ERR, EDAC_MC,
> + "Failed to create sysfs entries\n");
> + goto remove_sysfs;
> + }
> +
> + /*
> + * Firmware driver returns -ENODEV if it is not probed. In this case
> + * defer XilSEM error event registration.
> + */
> + rc = zynqmp_pm_get_family_info(&family_code);
> + if (rc) {
> + if (rc == -ENODEV)
> + rc = -EPROBE_DEFER;
> +
> + goto del_mc;
> + }
> + if (family_code == PM_VERSAL_FAMILY_CODE) {
> + priv->xsem_rtca->sw_event_node_id = VERSAL_EVENT_ERROR_SW_ERR;
> + priv->xsem_rtca->cram_ce_mask = XPM_VERSAL_EVENT_ERROR_MASK_XSEM_CRAM_CE_5;
> + priv->xsem_rtca->cram_ue_mask = XPM_VERSAL_EVENT_ERROR_MASK_XSEM_CRAM_UE_6;
> + priv->xsem_rtca->npi_ue_mask = XPM_VERSAL_EVENT_ERROR_MASK_XSEM_NPI_UE_7;
> + } else {
> + edac_printk(KERN_ERR, EDAC_MC, "Invalid Device family code %d\n", family_code);
> + }
> +
> + rc = xlnx_register_event(PM_NOTIFY_CB, priv->xsem_rtca->sw_event_node_id,
> + priv->xsem_rtca->cram_ce_mask | priv->xsem_rtca->cram_ue_mask |
> + priv->xsem_rtca->npi_ue_mask,
> + false, xsem_err_callback, mci);
> + if (rc)
> + goto del_mc;
> +
> #ifdef CONFIG_EDAC_DEBUG
> create_debugfs_attributes(mci);
> setup_address_map(priv);
> @@ -1154,6 +2484,8 @@ static int mc_probe(struct platform_device *pdev)
> enable_intr(priv);
> return rc;
>
> +remove_sysfs:
> + xsem_edac_remove_sysfs_attributes(mci);
> del_mc:
> edac_mc_del_mc(&pdev->dev);
> free_edac_mc:
> @@ -1173,9 +2505,21 @@ static void mc_remove(struct platform_device *pdev)
> debugfs_remove_recursive(priv->debugfs);
> #endif
>
> + /* Unregister XilSem events if they were registered */
> + if (priv->xsem_rtca) {
> + xlnx_unregister_event(PM_NOTIFY_CB, priv->xsem_rtca->sw_event_node_id,
> + priv->xsem_rtca->cram_ce_mask |
> + priv->xsem_rtca->cram_ue_mask |
> + priv->xsem_rtca->npi_ue_mask,
> + xsem_err_callback, mci);
> + }
> xlnx_unregister_event(PM_NOTIFY_CB, VERSAL_EVENT_ERROR_PMC_ERR1,
> XPM_EVENT_ERROR_MASK_DDRMC_CR |
> XPM_EVENT_ERROR_MASK_DDRMC_NCR, err_callback, mci);
> + /* Remove XilSem sysfs attributes if they were created */
> + if (priv->xsem_rtca)
> + xsem_edac_remove_sysfs_attributes(mci);
> +
> edac_mc_del_mc(&pdev->dev);
> edac_mc_free(mci);
> }
^ permalink raw reply
* Re: [PATCH 1/2] regulator: dt-bindings: rtq2208: Label mtp-sel-high property as deprecated
From: Conor Dooley @ 2026-06-25 16:13 UTC (permalink / raw)
To: cy_huang
Cc: Mark Brown, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
Liam Girdwood, Yoon Dong Min, edward_kim, devicetree,
linux-kernel
In-Reply-To: <d111d67d40cc182a2ab8c6292e4ffff9c7dad9ba.1782353659.git.cy_huang@richtek.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] dt-bindings: gpu: img,powervr-*: Add maintainer entries
From: Conor Dooley @ 2026-06-25 16:11 UTC (permalink / raw)
To: Matt Coster
Cc: imagination, linux-kernel, Alessio Belle, Luigi Santivetti,
Frank Binns, Brajesh Gupta, Alexandru Dadu, dri-devel, devicetree
In-Reply-To: <20260625-maintainer-updates-v1-3-35112b2f038e@imgtec.com>
[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]
On Thu, Jun 25, 2026 at 03:03:39PM +0100, Matt Coster wrote:
> These entries already exist in MAINTAINERS, add them here so downstream
> users of the bindings also see them.
They only "already exist" because of the patch before this. The commit
message should reflect that this is a functional change in
maintainership not a correction to match a longstanding thing.
pw-bot: changes-requested
Thanks,
Conor.
>
> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> ---
> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 4 +++-
> Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> index a1f54dbae3f3..91e4ff61b394 100644
> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> @@ -8,7 +8,9 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
> title: Imagination Technologies PowerVR and IMG Rogue GPUs
>
> maintainers:
> - - Frank Binns <frank.binns@imgtec.com>
> + - Matt Coster <matt.coster@imgtec.com>
> + - Alessio Belle <alessio.belle@imgtec.com>
> + - Luigi Santivetti <luigi.santivetti@imgtec.com>
>
> properties:
> compatible:
> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml
> index f5898b04381c..3e7df7344430 100644
> --- a/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml
> @@ -9,7 +9,9 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
> title: Imagination Technologies PowerVR SGX GPUs
>
> maintainers:
> - - Frank Binns <frank.binns@imgtec.com>
> + - Matt Coster <matt.coster@imgtec.com>
> + - Alessio Belle <alessio.belle@imgtec.com>
> + - Luigi Santivetti <luigi.santivetti@imgtec.com>
>
> properties:
> compatible:
>
> --
> 2.48.1
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH] dt-bindings: dma: xlnx,axi-dma: Restore xlnx,flush-fsync as u32
From: Suraj Gupta @ 2026-06-25 16:10 UTC (permalink / raw)
To: vkoul, Frank.Li, robh, krzk+dt, conor+dt, michal.simek,
radhey.shyam.pandey
Cc: dmaengine, devicetree, linux-arm-kernel, linux-kernel
The DT schema conversion incorrectly changed xlnx,flush-fsync from a u32
property to a boolean. The original binding documented values 1, 2, and 3
to select which VDMA channel(s) flush on frame sync.
Restore the uint32 type with the documented enum values and fix the
example accordingly.
Fixes: 2d5c2952b972 ("dt-bindings: dma: xlnx,axi-dma: Convert to DT schema")
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
---
Documentation/devicetree/bindings/dma/xilinx/xlnx,axi-dma.yaml | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/xilinx/xlnx,axi-dma.yaml b/Documentation/devicetree/bindings/dma/xilinx/xlnx,axi-dma.yaml
index 340ae9e91cb0..95b951eea1b7 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xlnx,axi-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/xilinx/xlnx,axi-dma.yaml
@@ -81,8 +81,13 @@ properties:
description: Should be the number of framebuffers as configured in h/w.
xlnx,flush-fsync:
- type: boolean
- description: Tells which channel to Flush on Frame sync.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2, 3]
+ description:
+ Tells which channel to flush on frame sync.
+ 1 - flush both channels
+ 2 - flush mm2s channel
+ 3 - flush s2mm channel
xlnx,sg-length-width:
$ref: /schemas/types.yaml#/definitions/uint32
@@ -251,7 +256,7 @@ examples:
"m_axi_s2mm_aclk", "m_axis_mm2s_aclk",
"s_axis_s2mm_aclk";
xlnx,num-fstores = <8>;
- xlnx,flush-fsync;
+ xlnx,flush-fsync = <1>;
xlnx,addrwidth = <32>;
dma-channel-mm2s {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] hwmon: (chipcap2) Add support for label
From: sashiko-bot @ 2026-06-25 16:08 UTC (permalink / raw)
To: Flaviu Nistor; +Cc: linux-hwmon, conor+dt, robh, devicetree
In-Reply-To: <20260625160423.17882-2-flaviu.nistor@gmail.com>
> Add support for label sysfs attribute similar to other hwmon devices.
> This is particularly useful for systems with multiple sensors on the
> same board, where identifying individual sensors is much easier since
> labels can be defined via device tree.
>
> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625160423.17882-1-flaviu.nistor@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix
From: sashiko-bot @ 2026-06-25 16:06 UTC (permalink / raw)
To: Shenwei Wang; +Cc: imx, conor+dt, robh, Frank.Li, devicetree
In-Reply-To: <20260625155432.815185-4-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Forcing global prefix matching on all rpmsg drivers without a 'longest-prefix match' rule can cause unintended device binding and collisions.
- [Medium] Module auto-loading via udev is broken for dynamically suffixed devices because module alias generation remains exact and lacks a trailing wildcard.
--
commit 7cf465d01393f57a212484ddaf8a931c40ec4179
Author: Shenwei Wang <shenwei.wang@nxp.com>
rpmsg: core: match rpmsg device IDs by prefix
This commit updates the rpmsg device matching logic to compare only the
length of the id->name string. This allows id_table entries to act as
prefixes, enabling drivers to bind to dynamically suffixed devices.
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index e7f7831d37f89..f95bfc9965d4c 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -414,7 +414,9 @@ ATTRIBUTE_GROUPS(rpmsg_dev);
> static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
> const struct rpmsg_device_id *id)
> {
> - return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
> + size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
> +
> + return strncmp(id->name, rpdev->id.name, len) == 0;
> }
[Severity: High]
Will this change cause unintended driver binding for existing drivers that
expect an exact match?
Because this modifies the global match behavior for all rpmsg drivers without
an opt-in flag or longest-prefix match logic, a shorter existing prefix could
hijack a new device.
For example, if a remote processor creates a channel named rpmsg-tty-control,
would it be incorrectly matched by an existing driver registered as rpmsg-tty?
Looking at rpmsg_dev_match(), the iteration returns on the first successful
match:
drivers/rpmsg/rpmsg_core.c:rpmsg_dev_match() {
...
for (i = 0; ids[i].name[0]; i++)
if (rpmsg_id_match(rpdev, &ids[i])) {
rpdev->id.driver_data = ids[i].driver_data;
return 1;
}
...
}
Could this result in a collision if multiple IDs share the same prefix?
[Severity: Medium]
Does this change break module auto-loading via udev for dynamically suffixed
devices?
When a dynamically suffixed device is registered, rpmsg_uevent() emits the
exact name:
drivers/rpmsg/rpmsg_core.c:rpmsg_uevent() {
...
return add_uevent_var(env, "MODALIAS=" RPMSG_DEVICE_MODALIAS_FMT,
rpdev->id.name);
}
However, the module alias generated by file2alias.c does not append a
wildcard for prefix matching:
scripts/mod/file2alias.c:do_rpmsg_entry() {
...
module_alias_printf(mod, false, RPMSG_DEVICE_MODALIAS_FMT, *name);
}
Since module_alias_printf() is called with append_wildcard set to false, the
generated alias will be an exact match (e.g., rpmsg:rpmsg-io-) which won't
match the uevent string (e.g., MODALIAS=rpmsg:rpmsg-io-1).
Will this prevent the module from loading automatically?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=3
^ permalink raw reply
* Re: [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
From: sashiko-bot @ 2026-06-25 16:06 UTC (permalink / raw)
To: Shenwei Wang; +Cc: devicetree, robh, Frank.Li, conor+dt, imx
In-Reply-To: <20260625155432.815185-6-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `rpmsg-gpio` nodes are nested too deeply in the device tree for the `gpio-rpmsg` driver to discover them, causing an unconditional probe failure.
--
commit 9072997f8f4ba10a42ddca328c65744333555ac3
Author: Shenwei Wang <shenwei.wang@nxp.com>
arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
This patch adds an RPMSG bus node and its GPIO subnodes to the device tree
for the i.MX platform. It aims to enable remote device communication and
GPIO control via RPMSG.
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index 1de3ad60c6aa7..f1b984eb1203a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> @@ -190,6 +190,31 @@ scmi_sensor: protocol@15 {
> cm33: remoteproc-cm33 {
> compatible = "fsl,imx8ulp-cm33";
> status = "disabled";
> +
> + rpmsg {
> + rpmsg-io {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rpmsg_gpioa: gpio@0 {
[Severity: Medium]
Does nesting these nodes cause the gpio-rpmsg driver to fail to probe?
Looking at rpmsg_find_child_by_compat_reg() in drivers/gpio/gpio-rpmsg.c,
the driver only searches the direct children of the remoteproc node:
for_each_available_child_of_node(parent, child) {
if (!of_device_is_compatible(child, compat))
continue;
Since for_each_available_child_of_node() is not recursive, the driver
fails to find the nested gpio@0 and gpio@1 nodes under rpmsg/rpmsg-io/.
This causes an -ENODEV return, preventing the driver from probing.
> + compatible = "rpmsg-gpio";
> + reg = <0>;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=5
^ permalink raw reply
* [PATCH v2 2/2] hwmon: (chipcap2) Add support for label
From: Flaviu Nistor @ 2026-06-25 16:04 UTC (permalink / raw)
To: Guenter Roeck, Javier Carrasco, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan
Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree, linux-doc
In-Reply-To: <20260625160423.17882-1-flaviu.nistor@gmail.com>
Add support for label sysfs attribute similar to other hwmon devices.
This is particularly useful for systems with multiple sensors on the
same board, where identifying individual sensors is much easier since
labels can be defined via device tree.
Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
Changes in v2:
- No change for this patch in the patch series.
- Link to v1: https://lore.kernel.org/all/20260622122200.14245-1-flaviu.nistor@gmail.com/
Documentation/hwmon/chipcap2.rst | 2 ++
drivers/hwmon/chipcap2.c | 25 +++++++++++++++++++++++--
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/hwmon/chipcap2.rst b/Documentation/hwmon/chipcap2.rst
index dc165becc64c..c38d87b91b69 100644
--- a/Documentation/hwmon/chipcap2.rst
+++ b/Documentation/hwmon/chipcap2.rst
@@ -70,4 +70,6 @@ humidity1_min_hyst: RW humidity low hystersis
humidity1_max_hyst: RW humidity high hystersis
humidity1_min_alarm: RO humidity low alarm indicator
humidity1_max_alarm: RO humidity high alarm indicator
+humidity1_label: RO descriptive name for the sensor
+temp1_label: RO descriptive name for the sensor
=============================== ======= ========================================
diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index 4aecf463180f..086571d556b7 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -22,6 +22,8 @@
#include <linux/irq.h>
#include <linux/module.h>
#include <linux/regulator/consumer.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
#define CC2_START_CM 0xA0
#define CC2_START_NOM 0x80
@@ -83,6 +85,7 @@ struct cc2_data {
struct i2c_client *client;
struct regulator *regulator;
const char *name;
+ const char *label;
int irq_ready;
int irq_low;
int irq_high;
@@ -449,6 +452,8 @@ static umode_t cc2_is_visible(const void *data, enum hwmon_sensor_types type,
switch (attr) {
case hwmon_humidity_input:
return 0444;
+ case hwmon_humidity_label:
+ return cc2->label ? 0444 : 0;
case hwmon_humidity_min_alarm:
return cc2->rh_alarm.low_alarm_visible ? 0444 : 0;
case hwmon_humidity_max_alarm:
@@ -466,6 +471,8 @@ static umode_t cc2_is_visible(const void *data, enum hwmon_sensor_types type,
switch (attr) {
case hwmon_temp_input:
return 0444;
+ case hwmon_temp_label:
+ return cc2->label ? 0444 : 0;
default:
return 0;
}
@@ -552,6 +559,16 @@ static int cc2_humidity_max_alarm_status(struct cc2_data *data, long *val)
return 0;
}
+static int cc2_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ struct cc2_data *data = dev_get_drvdata(dev);
+
+ *str = data->label;
+
+ return 0;
+}
+
static int cc2_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
int channel, long *val)
{
@@ -670,8 +687,9 @@ static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
}
static const struct hwmon_channel_info *cc2_info[] = {
- HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
- HWMON_CHANNEL_INFO(humidity, HWMON_H_INPUT | HWMON_H_MIN | HWMON_H_MAX |
+ HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
+ HWMON_CHANNEL_INFO(humidity, HWMON_H_INPUT | HWMON_H_LABEL |
+ HWMON_H_MIN | HWMON_H_MAX |
HWMON_H_MIN_HYST | HWMON_H_MAX_HYST |
HWMON_H_MIN_ALARM | HWMON_H_MAX_ALARM),
NULL
@@ -680,6 +698,7 @@ static const struct hwmon_channel_info *cc2_info[] = {
static const struct hwmon_ops cc2_hwmon_ops = {
.is_visible = cc2_is_visible,
.read = cc2_read,
+ .read_string = cc2_read_string,
.write = cc2_write,
};
@@ -710,6 +729,8 @@ static int cc2_probe(struct i2c_client *client)
return dev_err_probe(dev, PTR_ERR(data->regulator),
"Failed to get regulator\n");
+ device_property_read_string(dev, "label", &data->label);
+
ret = cc2_request_ready_irq(data, dev);
if (ret)
return dev_err_probe(dev, ret, "Failed to request ready irq\n");
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: hwmon: chipcap2: Add label property
From: Flaviu Nistor @ 2026-06-25 16:04 UTC (permalink / raw)
To: Guenter Roeck, Javier Carrasco, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan
Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree, linux-doc
Add support for an optional label property similar to other hwmon devices.
This allows, in case of boards with multiple CHIPCAP2 sensors, to assign
distinct names to each instance.
Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
Changes in v2:
- Implement suggestion from Javier Carrasco as proposed by Krzysztof Kozlowski.
- Link to v1: https://lore.kernel.org/all/20260622122200.14245-1-flaviu.nistor@gmail.com/
.../devicetree/bindings/hwmon/amphenol,chipcap2.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
index 17351fdbefce..56b0cecfca5f 100644
--- a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
+++ b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
@@ -45,6 +45,8 @@ properties:
- const: low
- const: high
+ label: true
+
vdd-supply:
description:
Dedicated, controllable supply-regulator to reset the device and
@@ -55,6 +57,9 @@ required:
- reg
- vdd-supply
+allOf:
+ - $ref: hwmon-common.yaml#
+
additionalProperties: false
examples:
@@ -72,6 +77,7 @@ examples:
<5 IRQ_TYPE_EDGE_RISING>,
<6 IRQ_TYPE_EDGE_RISING>;
interrupt-names = "ready", "low", "high";
+ label = "Room";
vdd-supply = <®_vdd>;
};
};
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
From: sashiko-bot @ 2026-06-25 16:04 UTC (permalink / raw)
To: Shenwei Wang; +Cc: devicetree, conor+dt, Frank.Li, imx, robh
In-Reply-To: <20260625155432.815185-5-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [Critical] Writing to the `.name` field of the immutable `gpio_rpmsg_irq_chip` structure attempts to modify read-only memory, causing a kernel panic.
- [High] Host-endian integers are directly assigned to little-endian protocol fields, breaking communication on big-endian architectures.
- [High] An RPMSG timeout permanently desynchronizes the request/response state machine because the driver lacks transaction IDs to discard late replies.
- [High] The RPMSG callback blindly accesses the incoming message without validating that the payload length is sufficient.
- [High] The driver's GPIO callbacks can sleep, but it fails to set `gc->can_sleep = true`, incorrectly allowing them to be called from atomic contexts.
- [High] Interrupts will be permanently masked after firing once because the driver incorrectly relies on `irq_bus_sync_unlock` to send the unmask command during normal IRQ flow.
- [Medium] The driver leaks a reference to the `rproc` parent device node.
- [Medium] `rpmsg_gpio_direction_output()` configures the GPIO line as an output before setting the desired logical value, potentially causing a signal glitch.
--
commit e50fe4c0d14d5e1abdf1ce315d5d9511d01ee66e
Author: Shenwei Wang <shenwei.wang@nxp.com>
Subject: gpio: rpmsg: add generic rpmsg GPIO driver
This commit adds a generic GPIO driver to support controlling GPIO
controllers that reside on a remote processor. It translates standard
Linux GPIO calls into RPMSG protocol messages sent to the remote side.
> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 0000000000000..332e2925a8309
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c
[ ... ]
> +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port)
> +{
> + int ret;
> +
> + reinit_completion(&port->cmd_complete);
> +
> + ret = rpmsg_send(port->rpdev->ept, port->send_msg, sizeof(*port->send_msg));
> + if (ret) {
> + dev_err(&port->rpdev->dev, "rpmsg_send failed: cmd=%d ret=%d\n",
> + port->send_msg->type, ret);
> + return ret;
> + }
> +
> + ret = wait_for_completion_timeout(&port->cmd_complete,
> + msecs_to_jiffies(RPMSG_TIMEOUT));
> + if (ret == 0) {
> + dev_err(&port->rpdev->dev, "rpmsg_send timeout! cmd=%d\n",
> + port->send_msg->type);
> + return -ETIMEDOUT;
[Severity: High]
If rpmsg_send times out, can this permanently desynchronize the request and
response state machine?
If a timeout occurs, rpmsg_gpio_send_message() returns -ETIMEDOUT but leaves
the channel active. Since the rpmsg_gpio_response doesn't appear to contain a
transaction ID, if the remote processor sends a delayed reply to this timed-out
command, will rpmsg_gpio_channel_callback() mistakenly accept it as the
completion for the next unrelated command sent by the host?
> + }
> +
> + if (unlikely(port->recv_msg->status != VIRTIO_GPIO_STATUS_OK)) {
> + dev_err(&port->rpdev->dev, "remote core replies an error: cmd=%d!\n",
> + port->send_msg->type);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static struct virtio_gpio_request *
> +rpmsg_gpio_msg_prepare(struct rpmsg_gpio_port *port, u16 line, u16 cmd, u32 val)
> +{
> + struct virtio_gpio_request *msg = port->send_msg;
> +
> + msg->type = cmd;
> + msg->gpio = line;
> + msg->value = val;
[Severity: High]
Are these values being assigned with the correct endianness?
The virtio_gpio_request structure defines these fields as __le16 and __le32,
but this assigns native host-endian integers to them directly. On big-endian
architectures, could this result in the remote processor misinterpreting the
commands?
> +
> + return msg;
> +}
[ ... ]
> +static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned int line, int val)
> +{
> + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + int ret;
> +
> + guard(mutex)(&port->lock);
> +
> + rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
> + VIRTIO_GPIO_DIRECTION_OUT);
> +
> + ret = rpmsg_gpio_send_message(port);
> + if (ret)
> + return ret;
> +
> + rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
[Severity: Medium]
Could configuring the direction before setting the value cause a signal
glitch?
rpmsg_gpio_direction_output() sends VIRTIO_GPIO_MSG_SET_DIRECTION to the
remote processor before VIRTIO_GPIO_MSG_SET_VALUE. If the remote hardware
applies the direction immediately, could this drive the line to its default
or previous state for a short window before the intended new value is applied?
> +
> + return rpmsg_gpio_send_message(port);
> +}
[ ... ]
> +static void gpio_rpmsg_unmask_irq(struct irq_data *d)
> +{
> + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> + u32 line = d->hwirq;
> +
> + port->lines[line].irq_unmask = 1;
> +}
[Severity: High]
Will interrupts remain permanently masked on the remote processor after they
fire once?
gpio_rpmsg_unmask_irq() updates local memory but does not send an RPMSG
command to unmask the interrupt remotely. While gpio_rpmsg_irq_bus_sync_unlock()
does send the command, the core IRQ subsystem (like handle_level_irq) masks
and unmasks interrupts without invoking bus lock/unlock callbacks during normal
interrupt dispatch. Does this mean the unmask command is never actually sent
after an interrupt is handled?
[ ... ]
> +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev,
> + struct device_node *np, const char *name)
> +{
> + struct rpmsg_gpio_port *port;
> + struct gpio_irq_chip *girq;
> + struct gpio_chip *gc;
> + int ret;
> +
> + port = devm_kzalloc(&rpdev->dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32(np, "reg", &port->idx);
> + if (ret)
> + return ret;
> +
> + ret = devm_mutex_init(&rpdev->dev, &port->lock);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_u32(np, "ngpios", &port->ngpios);
> + if (ret || port->ngpios > GPIOS_PER_PORT_DEFAULT)
> + port->ngpios = GPIOS_PER_PORT_DEFAULT;
> +
> + port->send_msg = devm_kzalloc(&rpdev->dev,
> + sizeof(*port->send_msg),
> + GFP_KERNEL);
> +
> + port->recv_msg = devm_kzalloc(&rpdev->dev,
> + sizeof(*port->recv_msg),
> + GFP_KERNEL);
> + if (!port->send_msg || !port->recv_msg)
> + return -ENOMEM;
> +
> + init_completion(&port->cmd_complete);
> + port->rpdev = rpdev;
> +
> + gc = &port->gc;
> + gc->owner = THIS_MODULE;
> + gc->parent = &rpdev->dev;
> + gc->fwnode = of_fwnode_handle(np);
> + gc->ngpio = port->ngpios;
> + gc->base = -1;
> + gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
> + name, port->idx);
[Severity: High]
Should gc->can_sleep be explicitly set to true for this gpio_chip?
The .set and .get callbacks in this driver acquire a mutex and wait for
completion, which will sleep. Without setting gc->can_sleep = true, the
gpiolib framework treats these GPIOs as atomic. If consumer drivers call
these GPIO operations from within an interrupt handler or a spinlock,
could this trigger a "scheduling while atomic" panic?
> +
> + gc->direction_input = rpmsg_gpio_direction_input;
> + gc->direction_output = rpmsg_gpio_direction_output;
> + gc->get_direction = rpmsg_gpio_get_direction;
> + gc->get = rpmsg_gpio_get;
> + gc->set = rpmsg_gpio_set;
> +
> + girq = &gc->irq;
> + gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->chip->name = devm_kstrdup(&rpdev->dev, gc->label, GFP_KERNEL);
[Severity: Critical]
Does this attempt to write to read-only memory?
gpio_irq_chip_set_chip() assigns the const structure gpio_rpmsg_irq_chip to
girq->chip. Modifying the .name field of a const structure placed in the
.rodata section will trigger a write-protection page fault and crash the
kernel. Should the irq_chip be dynamically allocated, or should the name be
set differently?
> +
> + dev_set_drvdata(&rpdev->dev, port);
> +
> + return devm_gpiochip_add_data(&rpdev->dev, gc, port);
> +}
[ ... ]
> +static struct device_node *
> +rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, const char *compat, u32 idx)
> +{
> + struct device_node *np_chan = NULL, *np;
> + struct rproc *rproc;
> +
> + rproc = rproc_get_by_child(&rpdev->dev);
> + if (!rproc)
> + return NULL;
> +
> + np = of_node_get(rproc->dev.of_node);
> + if (!np && rproc->dev.parent)
> + np = of_node_get(rproc->dev.parent->of_node);
> +
> + if (np)
> + np_chan = rpmsg_find_child_by_compat_reg(np, compat, idx);
> +
> + return np_chan;
[Severity: Medium]
Does this leak the reference to the parent device node?
rpmsg_get_channel_ofnode() acquires a reference to np using of_node_get(),
but it appears to return without calling of_node_put(np) to release it.
> +}
[ ... ]
> +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
> + int len, void *priv, u32 src)
> +{
> + struct rpmsg_gpio_response *msg = data;
> + struct rpmsg_gpio_port *port = NULL;
> +
> + port = dev_get_drvdata(&rpdev->dev);
> +
> + if (!port) {
> + dev_err(&rpdev->dev, "port is null\n");
> + return -EINVAL;
> + }
> +
> + if (msg->type == GPIO_RPMSG_REPLY) {
[Severity: High]
Is there a risk of an out-of-bounds read here if the message is truncated?
rpmsg_gpio_channel_callback() receives a payload of size len, but blindly
dereferences the data pointer as struct rpmsg_gpio_response without
verifying that len >= sizeof(struct rpmsg_gpio_response). Could this read
out-of-bounds memory if the remote processor sends a malformed or small
message?
> + *port->recv_msg = *msg;
> + complete(&port->cmd_complete);
> + } else if (msg->type == GPIO_RPMSG_NOTIFY) {
> + generic_handle_domain_irq_safe(port->gc.irq.domain, msg->line);
> + } else {
> + dev_err(&rpdev->dev, "wrong message type (0x%x)\n", msg->type);
> + }
> +
> + return 0;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=4
^ permalink raw reply
* Re: [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
From: sashiko-bot @ 2026-06-25 16:03 UTC (permalink / raw)
To: Shenwei Wang; +Cc: robh, conor+dt, devicetree, Frank.Li, imx
In-Reply-To: <20260625155432.815185-2-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Missing endianness specification and implementation for multi-byte protocol fields.
- [High] The driver's direction_output callback causes hardware glitches by setting the direction before the initial value.
- [Medium] Inconsistent field sizes for REPLY and NOTIFICATION messages truncate the 'line' field.
- [Low] Incorrect command code in the SET_WAKEUP request diagram.
- [Low] Incorrect Linux error code usage (-ETIMEOUT).
- [Low] Grammatical error in SET_WAKEUP description.
--
commit 7712498eeadf5080c07364375a209ec4b75b63fd
Author: Shenwei Wang <shenwei.wang@nxp.com>
docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
This commit introduces the documentation for the GPIO RPMSG transport
protocol used over the RPMSG bus. It details the message formats,
command codes, and notification structures for communicating with
remote processors.
> diff --git a/Documentation/driver-api/gpio/gpio-rpmsg.rst b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> new file mode 100644
> index 0000000000000..7d351ff0adb0e
> --- /dev/null
> +++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> @@ -0,0 +1,271 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +GPIO RPMSG (Remote Processor Messaging) Protocol
> +================================================
[ ... ]
> +Message Format
> +--------------
> +
> +The RPMSG message consists of a 8-byte packet with the following layout:
> +
> +.. code-block:: none
> +
> + +------+------+------+------+------+------+------+------+
> + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
> + | cmd | line | value |
> + +------+------+------+------+------+------+------+------+
[Severity: High]
Does this message format lack an explicit endianness specification for the
multi-byte fields (cmd, line, value)?
Without defining the endianness here, the driver implementation in
drivers/gpio/gpio-rpmsg.c:rpmsg_gpio_msg_prepare() assigns native integer types
directly:
msg->type = cmd;
msg->gpio = line;
msg->value = val;
Could this cause malformed packets when communicating between a big-endian host
and the remote processor?
> +
> +- **cmd**: Command code, used for GPIO_RPMSG_SEND messages.
[ ... ]
> +GPIO Commands
> +-------------
> +
> +Commands are specified in the **Cmd** field.
> +
> +The SEND message is always sent from Linux to the remote firmware. Each
> +SEND corresponds to a single REPLY message. The GPIO driver should
> +serialize messages and determine whether a REPLY message is required. If a
> +REPLY message is expected but not received within the specified timeout
> +period (currently 1 second in the Linux driver), the driver should return
> +-ETIMEOUT.
[Severity: Low]
Should this use the standard Linux kernel error code -ETIMEDOUT instead of
-ETIMEOUT?
> +
> +GET_DIRECTION (Cmd=2)
[ ... ]
> +SET_DIRECTION (Cmd=3)
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +**Request:**
> +
> +.. code-block:: none
> +
> + +------+------+------+------+------+------+------+------+
> + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
> + | 3 | line | value |
> + +------+------+------+------+------+------+------+------+
> +
> +- **value**: Direction.
> +
> + - 0: None
> + - 1: Output
> + - 2: Input
[Severity: High]
Does this command lack a parameter to specify the initial output level,
requiring a separate SET_VALUE call?
In drivers/gpio/gpio-rpmsg.c:rpmsg_gpio_direction_output(), the driver sends
the SET_DIRECTION command before the SET_VALUE command:
rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
VIRTIO_GPIO_DIRECTION_OUT);
ret = rpmsg_gpio_send_message(port);
if (ret)
return ret;
rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
return rpmsg_gpio_send_message(port);
Could this sequence cause a hardware glitch by driving an unintended default
state on the physical GPIO line during the IPC transit time, before the
SET_VALUE command is processed?
> +
> +**Reply:**
[ ... ]
> +SET_WAKEUP (Cmd=16)
> +~~~~~~~~~~~~~~~~~~~
> +
> +**Request:**
> +
> +.. code-block:: none
> +
> + +------+------+------+------+------+------+------+------+
> + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
> + | 1 | line | value |
> + +------+------+------+------+------+------+------+------+
[Severity: Low]
Does the command field in this diagram say '1' when it should say '16' or
'0x10' to match the SET_WAKEUP (Cmd=16) heading?
> +
> +- **value**: Wakeup enable.
> +
> + The remote system should always aim to stay in a power-efficient state by
> + shutting down or clock-gating the GPIO blocks that aren't in use. Since
> + the remoteproc driver is responsible for managing the power states of the
> + remote firmware, the GPIO driver does not require to know the firmware's
> + running states.
[Severity: Low]
Should "does not require to know" be rephrased to "does not need to know" for
better grammar?
> +
> + When the wakeup bit is set, the remote firmware should configure the line
[ ... ]
> +Notification Message
> +--------------------
> +
> +Notifications are sent by the remote core and they have
> +**Type=2 (GPIO_RPMSG_NOTIFY)**:
> +
> +When a GPIO line asserts an interrupt on the remote processor, the firmware
> +should immediately mask the corresponding interrupt source and send a
> +notification message to the Linux. Upon completion of the interrupt
> +handling on the Linux side, the driver should issue a
> +command **SET_IRQ_TYPE** to the firmware to unmask the interrupt.
> +
> +A Notification message can arrive between a SEND and its REPLY message,
> +and the driver is expected to handle this scenario.
> +
> +.. code-block:: none
> +
> + +------+------+--------+
> + | 0x00 | 0x01 | 0x02 |
> + | 2 | line | trigger|
> + +------+------+--------+
[Severity: Medium]
Does this notification message truncate the line index to 8 bits?
The SEND message diagram earlier explicitly defines an 8-byte layout with a
16-bit line field (occupying bytes 0x02 and 0x03). However, this NOTIFICATION
diagram and the driver's struct rpmsg_gpio_response in
drivers/gpio/gpio-rpmsg.c restrict the line index to an 8-bit __u8:
struct rpmsg_gpio_response {
...
__u8 line;
...
};
If an interrupt occurs on a line index greater than 255, could this cause the
driver to mishandle the IRQ due to truncation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=1
^ 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