* Re: [PATCH v3 5/8] dt-bindings: serial: cdns: add Axiado AX3005 UART variant
From: Krzysztof Kozlowski @ 2026-07-17 8:32 UTC (permalink / raw)
To: Swark Yang
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah,
Linus Walleij, Bartosz Golaszewski, Jan Kotas, Michal Simek,
Andi Shyti, Przemysław Gaj, Alexandre Belloni, Frank Li,
Boris Brezillon, Greg Kroah-Hartman, Jiri Slaby, Mark Brown,
Mathias Nyman, devicetree, linux-arm-kernel, linux-kernel,
linux-gpio, linux-i2c, linux-i3c, linux-serial, linux-spi,
linux-usb
In-Reply-To: <20260716-upstream-axiado-ax3005-upstream-v3-5-c429095143ec@axiado.com>
On Thu, Jul 16, 2026 at 08:51:12PM -0700, Swark Yang wrote:
> Add binding for Axiado AX3005 UART controller. So far, no changes
> are known, so it can fall back to the cdns,uart-r1p12 compatible.
>
> Signed-off-by: Swark Yang <syang@axiado.com>
> ---
> Documentation/devicetree/bindings/serial/cdns,uart.yaml | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v7 0/2] Enable audio support for J721S2 EVM
From: Moteen Shah @ 2026-07-17 8:31 UTC (permalink / raw)
To: krzk+dt, robh, conor+dt, nm, vigneshr, kristo, mturquette, sboyd,
bmasney
Cc: devicetree, linux-arm-kernel, linux-kernel, u-kumar1,
gehariprasath, y-abhilashchandra, m-shah, sen
Earlier version of the patchset sent upstream[0] was rejected as
its dependency[1], which resolves the DTBS check errors introduced
by [0] also got rejected on the grounds of ABI breakage.
Another solution to fix the DTBS check errors introduced by [0] is
to modify the ti,j721e-system-controller.yaml binding to allow
audio-refclk as clock-controller child. This is done in the first
patch of this series.
Changes since v6:
- Fix commit message in patch 2/2: split the I2C3 and McASP routing
bullet to correctly describe I2C3 routing via gpio-mux controller
and McASP serializer routing via GPIO hogs as separate items
- Add Reviewed-by from Sen
Changes since v5:
Link to v5:
https://lore.kernel.org/all/20260520115603.2662930-1-m-shah@ti.com/
- Move audio_refclk1 clock node from the overlay (k3-j721s2-evm-audio.dtso)
to the main DTSI (k3-j721s2-main.dtsi) so it is always available
independent of the overlay being applied
- Drop the &scm_conf block from the overlay since it is now in main.dtsi
Changes since v4:
Link to v4:
https://lore.kernel.org/all/20260519142341.2531948-1-m-shah@ti.com/
- Explicitly disable main_mcan3 and main_mcan5 as they will silently
break mcasp audio routing
Link to v3:
https://lore.kernel.org/all/20260330094459.128648-1-m-shah@ti.com/
Link to v2:
https://lore.kernel.org/all/20260205130707.2033197-1-m-shah@ti.com/
Link to v1:
https://lore.kernel.org/all/20260112104536.83309-1-m-shah@ti.com/
Bootlogs:
https://gist.github.com/Jamm02/f586087dad227450abe8637e8a688c68
[0]: https://lore.kernel.org/linux-arm-kernel/20250604104656.38752-1-j-choudhary@ti.com/
[1]: https://lore.kernel.org/all/20250603095609.33569-4-j-choudhary@ti.com/
Jayesh Choudhary (1):
arm64: dts: ti: Add audio overlay for k3-j721s2-evm
Moteen Shah (1):
dt-bindings: ti: Update audio-refclk binding and j721e system
controller
.../bindings/clock/ti,am62-audio-refclk.yaml | 8 +-
.../soc/ti/ti,j721e-system-controller.yaml | 6 +-
arch/arm64/boot/dts/ti/Makefile | 4 +
.../boot/dts/ti/k3-j721s2-evm-audio.dtso | 157 ++++++++++++++++++
arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 9 +
5 files changed, 180 insertions(+), 4 deletions(-)
create mode 100644 arch/arm64/boot/dts/ti/k3-j721s2-evm-audio.dtso
--
2.34.1
^ permalink raw reply
* [PATCH v7 1/2] dt-bindings: ti: Update audio-refclk binding and j721e system controller
From: Moteen Shah @ 2026-07-17 8:32 UTC (permalink / raw)
To: krzk+dt, robh, conor+dt, nm, vigneshr, kristo, mturquette, sboyd,
bmasney
Cc: devicetree, linux-arm-kernel, linux-kernel, u-kumar1,
gehariprasath, y-abhilashchandra, m-shah, sen
In-Reply-To: <20260717083201.1887640-1-m-shah@ti.com>
Add ti,j721s2-audio-refclk as a supported compatible string in the
ti,am62-audio-refclk binding. J721S2 uses the same audio reference
clock IP block first introduced on AM62. Per writing-bindings
guidelines, the J721S2-specific compatible is added as the primary
string with ti,am62-audio-refclk as the fallback.
Also extend the ti,j721e-system-controller clock-controller@ child
pattern to accept audio-refclk schemas alongside ehrpwm-tbclk via a
oneOf constraint, fixing the alphanumerical ordering of $refs.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Moteen Shah <m-shah@ti.com>
---
.../devicetree/bindings/clock/ti,am62-audio-refclk.yaml | 8 ++++++--
.../bindings/soc/ti/ti,j721e-system-controller.yaml | 6 ++++--
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/ti,am62-audio-refclk.yaml b/Documentation/devicetree/bindings/clock/ti,am62-audio-refclk.yaml
index b2e40bd39a3af..6c8fb0793070d 100644
--- a/Documentation/devicetree/bindings/clock/ti,am62-audio-refclk.yaml
+++ b/Documentation/devicetree/bindings/clock/ti,am62-audio-refclk.yaml
@@ -11,8 +11,12 @@ maintainers:
properties:
compatible:
- items:
- - const: ti,am62-audio-refclk
+ oneOf:
+ - items:
+ - const: ti,am62-audio-refclk
+ - items:
+ - const: ti,j721s2-audio-refclk
+ - const: ti,am62-audio-refclk
reg:
maxItems: 1
diff --git a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
index f3bd0be3b279f..364be49f8c7c7 100644
--- a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
+++ b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
@@ -53,9 +53,11 @@ patternProperties:
"^clock-controller@[0-9a-f]+$":
type: object
- $ref: /schemas/clock/ti,am654-ehrpwm-tbclk.yaml#
+ oneOf:
+ - $ref: /schemas/clock/ti,am62-audio-refclk.yaml#
+ - $ref: /schemas/clock/ti,am654-ehrpwm-tbclk.yaml#
description:
- Clock provider for TI EHRPWM nodes.
+ Clock provider for TI EHRPWM or Audio Reference Clock nodes.
"phy@[0-9a-f]+$":
type: object
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 4/8] dt-bindings: i3c: cdns: add Axiado AX3005 I3C variant
From: Krzysztof Kozlowski @ 2026-07-17 8:32 UTC (permalink / raw)
To: Swark Yang
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah,
Linus Walleij, Bartosz Golaszewski, Jan Kotas, Michal Simek,
Andi Shyti, Przemysław Gaj, Alexandre Belloni, Frank Li,
Boris Brezillon, Greg Kroah-Hartman, Jiri Slaby, Mark Brown,
Mathias Nyman, devicetree, linux-arm-kernel, linux-kernel,
linux-gpio, linux-i2c, linux-i3c, linux-serial, linux-spi,
linux-usb
In-Reply-To: <20260716-upstream-axiado-ax3005-upstream-v3-4-c429095143ec@axiado.com>
On Thu, Jul 16, 2026 at 08:51:11PM -0700, Swark Yang wrote:
> Add binding for Axiado AX3005 I3C master. So far, no changes
> are known, so it can fall back to the cdns,i3c-master compatible.
>
> Signed-off-by: Swark Yang <syang@axiado.com>
> ---
> Documentation/devicetree/bindings/i3c/cdns,i3c-master.yaml | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 3/8] dt-bindings: i2c: cdns: add Axiado AX3005 I2C variant
From: Krzysztof Kozlowski @ 2026-07-17 8:32 UTC (permalink / raw)
To: Swark Yang
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah,
Linus Walleij, Bartosz Golaszewski, Jan Kotas, Michal Simek,
Andi Shyti, Przemysław Gaj, Alexandre Belloni, Frank Li,
Boris Brezillon, Greg Kroah-Hartman, Jiri Slaby, Mark Brown,
Mathias Nyman, devicetree, linux-arm-kernel, linux-kernel,
linux-gpio, linux-i2c, linux-i3c, linux-serial, linux-spi,
linux-usb
In-Reply-To: <20260716-upstream-axiado-ax3005-upstream-v3-3-c429095143ec@axiado.com>
On Thu, Jul 16, 2026 at 08:51:10PM -0700, Swark Yang wrote:
> Add binding for Axiado AX3005 I2C controller. So far, no changes
> are known, so it can fall back to the cdns,i2c-r1p14 compatible.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 2/8] dt-bindings: gpio: cdns: add Axiado AX3005 GPIO variant
From: Krzysztof Kozlowski @ 2026-07-17 8:30 UTC (permalink / raw)
To: Swark Yang
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah,
Linus Walleij, Bartosz Golaszewski, Jan Kotas, Michal Simek,
Andi Shyti, Przemysław Gaj, Alexandre Belloni, Frank Li,
Boris Brezillon, Greg Kroah-Hartman, Jiri Slaby, Mark Brown,
Mathias Nyman, devicetree, linux-arm-kernel, linux-kernel,
linux-gpio, linux-i2c, linux-i3c, linux-serial, linux-spi,
linux-usb
In-Reply-To: <20260716-upstream-axiado-ax3005-upstream-v3-2-c429095143ec@axiado.com>
On Thu, Jul 16, 2026 at 08:51:09PM -0700, Swark Yang wrote:
> Add binding for Axiado AX3005 GPIO controller. So far, no changes
> are known, so it can fall back to the cdns,gpio-r1p02 compatible.
>
> Signed-off-by: Swark Yang <syang@axiado.com>
> ---
> Documentation/devicetree/bindings/gpio/cdns,gpio.yaml | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v8 4/9] nvmem: layouts: Support fixed-layout as the nvmem device node itself
From: Bartosz Golaszewski @ 2026-07-17 8:29 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-msm, linux-block,
linux-wireless, ath10k, linux-bluetooth, netdev, daniel,
Loic Poulain, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Jens Axboe,
Johannes Berg, Jeff Johnson, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Srinivas Kandagatla, Andrew Lunn, Heiner Kallweit,
Russell King, Saravana Kannan, Christian Marangi
In-Reply-To: <CAMRc=MdNehSRQT3MUs+QtbZsHcUCS8YjyZchTgHCVv-HaLQkeA@mail.gmail.com>
On Wed, 15 Jul 2026 14:07:24 +0200, Bartosz Golaszewski <brgl@kernel.org> said:
> On Fri, 3 Jul 2026 15:45:17 +0200, Loic Poulain <loic.poulain@oss.qualcomm.com> said:
>> of_nvmem_layout_get_container() only looks for a child node named
>> "nvmem-layout" to locate the cell definitions. This does not cover
>> providers whose device tree node is itself the fixed-layout container,
>> such as an eMMC boot partition block device whose fwnode points directly
>> at a "fixed-layout" compatible partitions node.
>>
>> When no "nvmem-layout" child is present, fall back to returning the nvmem
>> device node itself if it is compatible with "fixed-layout", so that its
>> cells are parsed by nvmem_add_cells_from_fixed_layout().
>>
>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>> ---
>> drivers/nvmem/layouts.c | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvmem/layouts.c b/drivers/nvmem/layouts.c
>> index b90584e1b99eab4217cbe7ec48373e18a7caf0dc..efa631ce7283bdd6c8ecda75915911b5e3a33c99 100644
>> --- a/drivers/nvmem/layouts.c
>> +++ b/drivers/nvmem/layouts.c
>> @@ -167,7 +167,18 @@ static int nvmem_layout_bus_populate(struct nvmem_device *nvmem,
>>
>> struct device_node *of_nvmem_layout_get_container(struct nvmem_device *nvmem)
>> {
>> - return of_get_child_by_name(nvmem->dev.of_node, "nvmem-layout");
>> + struct device_node *np;
>> +
>> + /* Search for nvmem-layout child */
>> + np = of_get_child_by_name(nvmem->dev.of_node, "nvmem-layout");
>> + if (np)
>> + return np;
>> +
>> + /* The nvmem of_node is itself a fixed-layout node */
>> + if (of_device_is_compatible(nvmem->dev.of_node, "fixed-layout"))
>> + return of_node_get(nvmem->dev.of_node);
>> +
>> + return NULL;
>> }
>> EXPORT_SYMBOL_GPL(of_nvmem_layout_get_container);
>>
>>
>> --
>> 2.34.1
>>
>>
>
> I have it on my TODO list to convert nvmem layouts to be fwnode-agnostic. While
> I'm not sure when I'll have the time to look into it, I think it makes sense
> to not introduce any new OF-specific interfaces. Can you make it into:
>
> struct fwnode_handle *nvmem_layout_get_container_node(struct nvmem_device *nvmem);
>
> by replacing of_get_child_by_name() with fwnode_get_name_child_node() and
> of_device_is_compatible() with device_is_compatible()? That would mean less
> churn in the future.
>
> Other than that, it looks good.
>
> Bart
>
Scratch that, I didn't realize this isn't a new function but a rework of an
existing one.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Bart
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: eliza-cqs-evk: Enable sound card support with WSA8845 and DMIC
From: Konrad Dybcio @ 2026-07-17 8:29 UTC (permalink / raw)
To: Ravi Hothi, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
In-Reply-To: <20260713121816.2728888-3-ravi.hothi@oss.qualcomm.com>
On 7/13/26 2:18 PM, Ravi Hothi wrote:
> Enable the sound card on the Eliza CQS EVK platform, including the
> WSA8845 external speaker path and DMIC microphone capture via VA macro.
>
> Enable the required LPASS WSA macro, VA macro and SoundWire controller
> along with the necessary pinctrl configurations for DMIC and WSA
> SoundWire interfaces.
>
> Signed-off-by: Ravi Hothi <ravi.hothi@oss.qualcomm.com>
> ---
[...]
> + sound {
> + compatible = "qcom,eliza-sndcard", "qcom,sm8450-sndcard";
> + model = "eliza-cqs-evk";
Is there a reason only the CQS EVK gets audio support?
i.e. is this setup not common across all the EVKs?
> +
> + audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
> + "SpkrRight IN", "WSA_SPK2 OUT",
> + "VA DMIC0", "vdd-micb",
> + "VA DMIC1", "vdd-micb",
> + "VA DMIC2", "vdd-micb",
> + "VA DMIC3", "vdd-micb",
> + "VA DMIC4", "vdd-micb",
> + "VA DMIC5", "vdd-micb",
> + "VA DMIC6", "vdd-micb",
> + "VA DMIC7", "vdd-micb";
Are there actually 8 DMICs?
> +
> + va-dai-link {
> + link-name = "VA Capture";
> +
> + cpu {
> + sound-dai = <&q6apmbedai VA_CODEC_DMA_TX_0>;
> + };
> +
> + codec {
'co'dec < 'cp'u
[...]
> + dmic_eldo: regulator-dmic-eldo {
> + compatible = "regulator-fixed";
> + regulator-name = "dmic-eldo";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + gpio = <&pm7550_gpios 8 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + vin-supply = <&vreg_bob>;
> + regulator-boot-on;
Is it really boot-on?
> + };
> +};
> +
> +&tlmm {
Label references should be sorted alphabetically
> + spkr_1_sd_n_active: spkr-1-sd-n-active-state {
> + pins = "gpio59";
> + function = "gpio";
> + drive-strength = <16>;
> + bias-disable;
> + output-low;
TLMM pins should not assert output state in the pinconf, the driver will
take care of that through the GPIOD APIs
On a note, these normally go through LPASS_TLMM, but I'm going to guess
these pins are not muxable to ADSP?
Konrad
^ permalink raw reply
* Re: [PATCH v3 3/4] dt-bindings: pinctrl: qcom,hawi-lpass-lpi-pinctrl: Add Hawi LPI pinctrl
From: Krzysztof Kozlowski @ 2026-07-17 8:26 UTC (permalink / raw)
To: Prasad Kumpatla
Cc: Bjorn Andersson, Bartosz Golaszewski, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Srinivas Kandagatla, Jaroslav Kysela, Takashi Iwai,
Srinivas Kandagatla, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, linux-sound
In-Reply-To: <20260715133243.3170821-4-prasad.kumpatla@oss.qualcomm.com>
On Wed, Jul 15, 2026 at 07:02:42PM +0530, Prasad Kumpatla wrote:
> Add bindings for the pin controller in Low Power Audio SubSystem (LPASS)
> of Qualcomm Hawi SoC.
>
> The controller has 23 GPIOs (gpio0-gpio22) with functions including
> SoundWire, I2S, DMIC, VA I2S, ext_mclk1, slimbus and lpass_lpi_dbg_clk.
>
> Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
> ---
> .../pinctrl/qcom,hawi-lpass-lpi-pinctrl.yaml | 109 ++++++++++++++++++
> 1 file changed, 109 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,hawi-lpass-lpi-pinctrl.yaml
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/2] arm64: dts: qcom: eliza: Add LPASS macro and SoundWire support
From: Konrad Dybcio @ 2026-07-17 8:25 UTC (permalink / raw)
To: Ravi Hothi, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
In-Reply-To: <20260713121816.2728888-2-ravi.hothi@oss.qualcomm.com>
On 7/13/26 2:18 PM, Ravi Hothi wrote:
> Add SoC-level LPASS WSA macro, VA macro, SoundWire controller and LPASS
> LPI pin controller nodes. DMIC and WSA SoundWire pinctrl states are
> defined inside the LPASS LPI pin controller node. The hardware is similar
> to the SM8750 platform.
>
> The WSA macro, VA macro and SoundWire controller are kept disabled so
> board DTS files can selectively enable and configure them.
>
> Signed-off-by: Ravi Hothi <ravi.hothi@oss.qualcomm.com>
> ---
[...]
> + #address-cells = <2>;
> + #size-cells = <0>;
> + #sound-dai-cells = <1>;
> +
> + status = "disabled";
> + };
> lpass_lpiaon_noc: interconnect@7400000 {
Missing \n above
Konrad
^ permalink raw reply
* Re: [PATCH v3 1/4] ASoC: dt-bindings: qcom: add LPASS LPR vote clock ID
From: Krzysztof Kozlowski @ 2026-07-17 8:24 UTC (permalink / raw)
To: Prasad Kumpatla
Cc: Bjorn Andersson, Bartosz Golaszewski, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Srinivas Kandagatla, Jaroslav Kysela, Takashi Iwai,
Srinivas Kandagatla, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, linux-sound, Bartosz Golaszewski
In-Reply-To: <ea6d2f6e-977f-413a-86b9-faf1bc2a8e23@oss.qualcomm.com>
On Thu, Jul 16, 2026 at 08:16:33PM +0530, Prasad Kumpatla wrote:
> > > #define LPASS_HW_DCODEC_VOTE 103
> > > +#define LPASS_HW_LPR_VOTE 104
> > > -#define Q6AFE_MAX_CLK_ID 104
> > > +#define Q6AFE_MAX_CLK_ID 105
> > Nope, ABI break. You cannot change that and as stated many times already
> > - if you need it, then it is not a binding.
>
> Hi Krzysztof,
>
> Thanks for the review.
>
> LPASS_HW_LPR_VOTE is intended to be consumed via the DT clock
> specifier, similar to LPASS_HW_MACRO_VOTE and LPASS_HW_DCODEC_VOTE.
> My intention was therefore to add a new DT-visible clock ID.
>
> Regarding the ABI concern, would assigning LPASS_HW_LPR_VOTE from
> the currently unused range (for example 100) and keeping
> Q6AFE_MAX_CLK_ID unchanged address the issue, or is the concern
> that LPASS_HW_LPR_VOTE should not be exposed as a DT binding at all?
> If not, could you please advise the appropriate place/example for
> defining this CLK ID?
Just drop the Q6AFE_MAX_CLK_ID from the bindings. I do not see it even
being used in the drivers, so no need to move it (unless you are aware
of some out-of-tree user).
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v7 04/13] dt-bindings: clk: zte: Add zx297520v3 LSP clock and reset controller
From: Krzysztof Kozlowski @ 2026-07-17 8:23 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, Vinod Koul,
Neil Armstrong, Russell King, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, linux-phy
In-Reply-To: <20260717-zx29clk-v7-4-408411cfcf36@gmail.com>
On Fri, Jul 17, 2026 at 12:35:40AM +0300, Stefan Dösinger wrote:
> The clock controller of the zx297520v3 Low Speed Peripherals is
> relatively clean. One register per device with gates, muxes and resets
> and for some devices a divider. There are even bits in the matrix
> controller to control propagation of clock lines down to LSP.
>
> The clocks are sorted by register address and I am convinced that the
> device list is complete. There are however a few more registers that are
> likely extra dividers for TDM and I2S devices
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* RE: [PATCH v20 00/12] Add Renesas RZ/G3L SD/eMMC support
From: Biju Das @ 2026-07-17 8:20 UTC (permalink / raw)
To: biju.das.au, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Philipp Zabel, magnus.damm
Cc: wsa+renesas, linux-mmc@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, Prabhakar Mahadev Lad,
biju.das.au
In-Reply-To: <20260713175159.138334-1-biju.das.jz@bp.renesas.com>
Hi All,
I got an update from hardware team for CLK divider bits for HS400 mode.
This change will incorporate in next version of hardware manual.
For HS400 mode:
00 0000 0000b: (SDxφ/2)/2
Other settings are prohibited.
Cheers,
Biju
> -----Original Message-----
> From: Biju <biju.das.au@gmail.com>
> Sent: 13 July 2026 18:52
> To: Ulf Hansson <ulfh@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Geert Uytterhoeven <geert+renesas@glider.be>;
> Philipp Zabel <p.zabel@pengutronix.de>; magnus.damm <magnus.damm@gmail.com>
> Cc: Biju Das <biju.das.jz@bp.renesas.com>; wsa+renesas <wsa+renesas@sang-engineering.com>; linux-
> mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; biju.das.au
> <biju.das.au@gmail.com>
> Subject: [PATCH v20 00/12] Add Renesas RZ/G3L SD/eMMC support
>
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> Hi All,
>
> RZ/G3L SoC has:
>
> Channel 0 supports SD and eMMC (including HS400/HS400ES).
> Channel 1 supports SD and eMMC (except for HS400).
> Channel 2 supports SD.
>
> The SoC supports a maximum frequency of 150 MHz. The SD0 interface does not support IOVS and PWEN in the
> SDHI register (no internal regulator), unlike SD1 and SD2. It has an internal divider for all modes
> except HS400.
> It also has a 2048-bit divider compared to 512 on others. Moreover RZ/G3L supports HS400 enhanced strobe
> mode.
>
> v19->v20:
> * Replaced the check mmd->clk_mask with mmc_data->clk_mask and moved
> the code after assignment of variable mmd, this ensures assigning
> the default values for non-DT platforms and DT platforms with no
> device data.
> * Replaced the check mmd->max_divider with mmc_data->max_divider and
> moved the code after assignment of variable mmd, this ensures
> assigning the default values for non-DT platforms and DT platforms with
> no device data.
> * Fixed the ordering of resets in suspend/resume paths.
> * Added bitfield.h header file.
> * Reworked on renesas_sdhi_set_clock() to handle the 11-bit divider
> case and the 32-bit register write.
> * Updated resume() with scoped PM runtime call.
> * SD_CLK_CTRL clk enable turned off before updating SCC_CKSEL_DTSEL
> register.
> v18->v19:
> * Updated commit description for patch#1,#2,#6,#8,#11 and #12
> * Updated clock and reset description with AXI master and AXI slave
> * Added else condition for reset and reset-names.
> * Renamed aclk->aclkm and reordered the axi clocks similar to resets.
> * Retained the tag got binding patch as the changes are trivial.
> * Fixed the clk_mask for non-DT platforms.
> * Fixed max-divider setting for non-DT platforms.
> * Replaced the magic number '9' with ilog2 function in
> renesas_sdhi_clk_enable().
> * Dropped divider variable from struct renesas_sdhi.
> * Updated renesas_sdhi_clk_update() to return rate for HS400 mode
> and non HS400 mode(uses 1/2 internal divider).
> * Updated handling for clk divider.
> * Renamed TMIO_MMC_HWADJ2->TMIO_MMC_HWADJ to make it generic for
> hardware tuning delays
> * Dropped duplicate SH_MOBILE_SDHI_SCC_TMPPORT2 macro
> * Updated suspend/resume with scoped runtime calls in suspend.
> * Fixed extra space in HS400MODE2 comment block.
> * Updated the comment HS400mode2->HS400MODE2.
> * Dropped the updation of clk handling as it is taken care in
> previous patches.
> * HS400ES support is enabled based on of_data.
> * Fixed the space in HS400ES comment block.
>
> v17->v18:
> * Collected tag
> * Merged patch #4 and #5 and updated commit description
> * Annotated the empty sentinel entries in the OF match tables with a
> "Sentinel." comment for clarity.
> * Retained the tag as it is a trivial cleanup.
> * New patches drop struct renesas_sdhi_hw_info, instead using
> renesas_sdhi_of_data and tmio_mmc_data.
> * Dropped clk, pinctrl, SoC, and board dtsi from this patch series;
> will send later.
> v1->v17:
> * Collected tag for binding patch.
> * Resending the series as there is an issue with patch threading from
> patch #14.
>
> Biju Das (12):
> dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC
> mmc: renesas_sdhi: Clean up whitespace and add OF table sentinels
> mmc: renesas_sdhi: Add clk_mask field to support flexible clock
> divider widths
> mmc: renesas_sdhi: Add max_divider field to support SoC-specific clock
> ranges
> mmc: renesas_sdhi: Add tuning delay support for RZ/G2L
> mmc: renesas_sdhi: Add TMIO_MMC_INTERNAL_DIVIDER flag
> mmc: renesas_sdhi: Add optional axis/axim reset controls
> mmc: renesas_sdhi: Add RZ/G3L SDHI support
> mmc: renesas_sdhi: Save and restore IOVS across suspend/resume
> mmc: renesas_sdhi: Make HS400 OSEL bit configurable per SoC
> mmc: renesas_sdhi: Add RZ/G3L HS400 support
> mmc: renesas_sdhi: Add HS400 enhanced strobe support for RZ/G3L
>
> .../devicetree/bindings/mmc/renesas,sdhi.yaml | 108 +++++--
> drivers/mmc/host/renesas_sdhi.h | 11 +-
> drivers/mmc/host/renesas_sdhi_core.c | 274 ++++++++++++++----
> drivers/mmc/host/renesas_sdhi_internal_dmac.c | 73 ++++-
> drivers/mmc/host/renesas_sdhi_sys_dmac.c | 12 +-
> include/linux/platform_data/tmio.h | 18 ++
> 6 files changed, 408 insertions(+), 88 deletions(-)
>
> --
> 2.43.0
^ permalink raw reply
* Re: [PATCH v3 7/7] clk: tests: Add Kunit testing for nexus nodes
From: Miquel Raynal @ 2026-07-17 8:19 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, conor+dt, devicetree, robh
In-Reply-To: <20260716114235.507CD1F00A3A@smtp.kernel.org>
On 16/07/2026 at 11:42:34 GMT, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The `of_find_node_by_name_kunit` helper creates a
> double-free vulnerability if a KUnit-managed node is passed as the
> `from` argument.
This is exactly what we want, since we want the exact same semantics as
of_find_node_by_name(), so: won't fix.
> - [Low] The `of_find_node_by_name_kunit` kernel-doc incorrectly states
> the return value is only a node or negative errno, omitting the NULL
> case when a node is not found.
I will mention the NULL possibility in the kernel doc, which is indeed
missing. This is what can happen if the lookup fails and this is the
value we want to return. The test case is already correctly checking the
output value.
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH v12 3/3] media: nxp: Add i.MX95 CSI pixel formatter v4l2 driver
From: Loic Poulain @ 2026-07-17 8:18 UTC (permalink / raw)
To: guoniu.zhou
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Laurent Pinchart, Frank Li, Abel Vesa, Peng Fan,
Michael Turquette, Stephen Boyd, imx, linux-media, devicetree,
linux-arm-kernel, linux-kernel, linux-clk, Guoniu Zhou
In-Reply-To: <20260716-csi_formatter-v12-3-2601d17fc4af@oss.nxp.com>
Hi Guoniu,
On Thu, Jul 16, 2026 at 9:11 AM <guoniu.zhou@oss.nxp.com> wrote:
>
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
>
> The CSI pixel formatter is a module found on i.MX95 used to reformat
> packet info, pixel and non-pixel data from CSI-2 host controller to
> match Pixel Link(PL) definition.
>
> Add data formatting support.
>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
> Changes in v12:
> - Fix stream ID handling: iterate routing table instead of assuming
> stream ID equals loop index (0-7)
> - Remove stream_to_vc[] array: derive VC from routing table and frame
> descriptor on each start/stop operation
> - Remove V4L2_SUBDEV_FL_HAS_EVENTS flag since driver does not generate events
> - Support stream IDs 0-63 by using BIT_ULL() for stream masks
> - Add get_frame_desc call in stop_stream with proper error handling
> - Add csi_formatter_read() helper function for register reads
> - Use read-modify-write for CSI_VC_PIXEL_DATA_TYPE register to support
> multiplexed streams sharing the same virtual channel
> - Use route->sink_pad instead of hardcoded CSI_FORMATTER_PAD_SINK
> - Write back coerced format in set_fmt before propagating to source stream
> - Drop Frank's Reviewed-by tag due to significant changes, requesting re-review
>
> Changes in v10:
> - Use u8 for vc in csi_formatter_get_vc() and drop vc < 0 check
> - Add MFD_SYSCON dependency to Kconfig
> - Fix stream/VC mapping potential mismatch in start/stop_stream functions
>
> Changes in v8:
> - Remove fmt field and look up format from subdev state instead
> - Unify function and structure naming to use csi_formatter_ prefix
> - Remove misleading alignment comment from set_fmt function
> - Optimize get_frame_desc to call once per start_stream
> - Replace V4L2_FRAME_DESC_ENTRY_MAX with CSI_FORMATTER_VC_NUM in loops
> - Remove redundant debug message in enable_streams
> - Use MEDIA_PAD_FL_MUST_CONNECT flag instead of manual link check
> - Fix typo: Formater -> Formatter in Kconfig help text
> - Improve grammar in data type index mapping comment
>
> Changes in v7:
> - Update references from imx9 to imx95 for consistency with dt-bindings
> - Enable PM runtime before async registration
>
> Changes in v6:
> - Remove unused header includes
> - Unify macro naming: VCx/VCX -> VC and parameter x -> vc
> - Remove unused format field from csi_formatter struct
> - Use compact initialization for formats array
> - Make find_csi_format() return NULL instead of default format
> - Use unsigned int for array index in find_csi_format()
> - Add err_ prefix to error handling labels
> - Add v4l2_subdev_cleanup() and reorder cleanup sequence
> - Update enable_streams debug output format
> - Rename VC_MAX to VC_NUM and fix boundary check
> - Update CSI formatter Kconfig description
> - Use v4l2_subdev_get_frame_desc_passthrough() helper
> - Fix error paths in async registration and probe
> - Add mutex to protect enabled_streams
> - Switch to devm_pm_runtime_enable()
> - Remove redundant num_routes check in set_routing
> - Optimize get_index_by_dt() and add warning for unsupported type
> - csi_formatter_start/stop_stream: Process all streams in mask
> ---
> MAINTAINERS | 8 +
> drivers/media/platform/nxp/Kconfig | 15 +
> drivers/media/platform/nxp/Makefile | 1 +
> drivers/media/platform/nxp/imx95-csi-formatter.c | 808 +++++++++++++++++++++++
> 4 files changed, 832 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index efbf808063e5..05009228b162 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19275,6 +19275,14 @@ S: Maintained
> F: Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> F: drivers/media/platform/nxp/imx-jpeg
>
> +NXP i.MX 95 CSI PIXEL FORMATTER V4L2 DRIVER
> +M: Guoniu Zhou <guoniu.zhou@nxp.com>
> +L: imx@lists.linux.dev
> +L: linux-media@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml
> +F: drivers/media/platform/nxp/imx95-csi-formatter.c
> +
> NXP i.MX CLOCK DRIVERS
> M: Abel Vesa <abelvesa@kernel.org>
> R: Peng Fan <peng.fan@nxp.com>
> diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
> index 40e3436669e2..8f49908b0022 100644
> --- a/drivers/media/platform/nxp/Kconfig
> +++ b/drivers/media/platform/nxp/Kconfig
> @@ -28,6 +28,21 @@ config VIDEO_IMX8MQ_MIPI_CSI2
> Video4Linux2 driver for the MIPI CSI-2 receiver found on the i.MX8MQ
> SoC.
>
> +config VIDEO_IMX95_CSI_FORMATTER
> + tristate "NXP i.MX95 CSI Pixel Formatter driver"
> + depends on ARCH_MXC || COMPILE_TEST
> + depends on MFD_SYSCON
> + depends on VIDEO_DEV
> + select MEDIA_CONTROLLER
> + select V4L2_FWNODE
> + select VIDEO_V4L2_SUBDEV_API
> + help
> + This driver provides support for the CSI Pixel Formatter found on
> + i.MX95 series SoCs. This module unpacks the pixels received from the
> + CSI-2 interface and reformats them to meet pixel link requirements.
> +
> + Say Y here to enable CSI Pixel Formatter module for i.MX95 SoC.
> +
> config VIDEO_IMX_MIPI_CSIS
> tristate "NXP MIPI CSI-2 CSIS receiver found on i.MX7 and i.MX8 models"
> depends on ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/media/platform/nxp/Makefile b/drivers/media/platform/nxp/Makefile
> index 4d90eb713652..6410115d870e 100644
> --- a/drivers/media/platform/nxp/Makefile
> +++ b/drivers/media/platform/nxp/Makefile
> @@ -6,6 +6,7 @@ obj-y += imx8-isi/
>
> obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
> +obj-$(CONFIG_VIDEO_IMX95_CSI_FORMATTER) += imx95-csi-formatter.o
> obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
> obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
> obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
> diff --git a/drivers/media/platform/nxp/imx95-csi-formatter.c b/drivers/media/platform/nxp/imx95-csi-formatter.c
> new file mode 100644
> index 000000000000..cea60327c972
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx95-csi-formatter.c
> @@ -0,0 +1,808 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025 NXP
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +
> +#include <media/mipi-csi2.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +/* CSI Pixel Formatter registers map */
> +
> +#define CSI_VC_INTERLACED_LINE_CNT(vc) (0x00 + (vc) * 0x04)
> +#define INTERLACED_ODD_LINE_CNT_SET(x) FIELD_PREP(GENMASK(13, 0), (x))
> +#define INTERLACED_EVEN_LINE_CNT_SET(x) FIELD_PREP(GENMASK(29, 16), (x))
> +
> +#define CSI_VC_INTERLACED_CTRL 0x20
> +
> +#define CSI_VC_INTERLACED_ERR 0x24
> +#define CSI_VC_ERR_MASK GENMASK(7, 0)
> +#define CSI_VC_ERR(vc) BIT((vc))
> +
> +#define CSI_VC_YUV420_FIRST_LINE_EVEN 0x28
> +#define YUV420_FIRST_LINE_EVEN(vc) BIT((vc))
> +
> +#define CSI_RAW32_CTRL 0x30
> +#define CSI_VC_RAW32_MODE(vc) BIT((vc))
> +#define CSI_VC_RAW32_SWAP_MODE(vc) BIT((vc) + 8)
> +
> +#define CSI_STREAM_FENCING_CTRL 0x34
> +#define CSI_VC_STREAM_FENCING(vc) BIT((vc))
> +#define CSI_VC_STREAM_FENCING_RST(vc) BIT((vc) + 8)
> +
> +#define CSI_STREAM_FENCING_STS 0x38
> +#define CSI_STREAM_FENCING_STS_MASK GENMASK(7, 0)
> +
> +#define CSI_VC_NON_PIXEL_DATA_TYPE(vc) (0x40 + (vc) * 0x04)
> +
> +#define CSI_VC_PIXEL_DATA_CTRL(vc) (0x60 + (vc) * 0x04)
> +#define NEW_VC(vc) FIELD_PREP(GENMASK(3, 1), vc)
> +#define REROUTE_VC_ENABLE BIT(0)
> +
> +#define CSI_VC_ROUTE_PIXEL_DATA_TYPE(vc) (0x80 + (vc) * 0x04)
> +
> +#define CSI_VC_NON_PIXEL_DATA_CTRL(vc) (0xa0 + (vc) * 0x04)
> +
> +#define CSI_VC_PIXEL_DATA_TYPE(vc) (0xc0 + (vc) * 0x04)
> +
> +#define CSI_VC_PIXEL_DATA_TYPE_ERR(vc) (0xe0 + (vc) * 0x04)
> +
> +#define CSI_FORMATTER_PAD_SINK 0
> +#define CSI_FORMATTER_PAD_SOURCE 1
> +#define CSI_FORMATTER_PAD_NUM 2
> +
> +#define CSI_FORMATTER_VC_NUM 8 /* Number of virtual channels */
> +
> +struct csi_formatter_pix_format {
> + u32 code;
> + u32 data_type;
> +};
> +
> +struct csi_formatter {
> + struct device *dev;
> + struct regmap *regs;
> + struct clk *clk;
> +
> + struct v4l2_subdev sd;
> + struct v4l2_subdev *csi_sd;
> + struct v4l2_async_notifier notifier;
> + struct media_pad pads[CSI_FORMATTER_PAD_NUM];
> +
> + u32 remote_pad;
> + u32 reg_offset;
> +
> + /* Protects enabled_streams */
> + struct mutex lock;
> + u64 enabled_streams;
> +};
> +
> +struct csi_formatter_dt_index {
> + u8 dtype;
> + u8 index;
> +};
> +
> +/*
> + * The index corresponds to the bit index in the register that enables
> + * the data type of pixel data transported by the Formatter.
> + */
> +static const struct csi_formatter_dt_index formatter_dt_to_index_map[] = {
> + { .dtype = MIPI_CSI2_DT_YUV420_8B, .index = 0 },
> + { .dtype = MIPI_CSI2_DT_YUV420_8B_LEGACY, .index = 2 },
> + { .dtype = MIPI_CSI2_DT_YUV422_8B, .index = 6 },
> + { .dtype = MIPI_CSI2_DT_RGB444, .index = 8 },
> + { .dtype = MIPI_CSI2_DT_RGB555, .index = 9 },
> + { .dtype = MIPI_CSI2_DT_RGB565, .index = 10 },
> + { .dtype = MIPI_CSI2_DT_RGB666, .index = 11 },
> + { .dtype = MIPI_CSI2_DT_RGB888, .index = 12 },
> + { .dtype = MIPI_CSI2_DT_RAW6, .index = 16 },
> + { .dtype = MIPI_CSI2_DT_RAW7, .index = 17 },
> + { .dtype = MIPI_CSI2_DT_RAW8, .index = 18 },
> + { .dtype = MIPI_CSI2_DT_RAW10, .index = 19 },
> + { .dtype = MIPI_CSI2_DT_RAW12, .index = 20 },
> + { .dtype = MIPI_CSI2_DT_RAW14, .index = 21 },
> + { .dtype = MIPI_CSI2_DT_RAW16, .index = 22 },
> +};
> +
> +static const struct csi_formatter_pix_format formats[] = {
> + /* YUV formats */
> + { MEDIA_BUS_FMT_UYVY8_1X16, MIPI_CSI2_DT_YUV422_8B },
> + /* RGB formats */
> + { MEDIA_BUS_FMT_RGB565_1X16, MIPI_CSI2_DT_RGB565 },
> + { MEDIA_BUS_FMT_RGB888_1X24, MIPI_CSI2_DT_RGB888 },
> + /* RAW (Bayer and greyscale) formats */
> + { MEDIA_BUS_FMT_SBGGR8_1X8, MIPI_CSI2_DT_RAW8 },
> + { MEDIA_BUS_FMT_SGBRG8_1X8, MIPI_CSI2_DT_RAW8 },
> + { MEDIA_BUS_FMT_SGRBG8_1X8, MIPI_CSI2_DT_RAW8 },
> + { MEDIA_BUS_FMT_SRGGB8_1X8, MIPI_CSI2_DT_RAW8 },
> + { MEDIA_BUS_FMT_Y8_1X8, MIPI_CSI2_DT_RAW8 },
> + { MEDIA_BUS_FMT_SBGGR10_1X10, MIPI_CSI2_DT_RAW10 },
> + { MEDIA_BUS_FMT_SGBRG10_1X10, MIPI_CSI2_DT_RAW10 },
> + { MEDIA_BUS_FMT_SGRBG10_1X10, MIPI_CSI2_DT_RAW10 },
> + { MEDIA_BUS_FMT_SRGGB10_1X10, MIPI_CSI2_DT_RAW10 },
> + { MEDIA_BUS_FMT_Y10_1X10, MIPI_CSI2_DT_RAW10 },
> + { MEDIA_BUS_FMT_SBGGR12_1X12, MIPI_CSI2_DT_RAW12 },
> + { MEDIA_BUS_FMT_SGBRG12_1X12, MIPI_CSI2_DT_RAW12 },
> + { MEDIA_BUS_FMT_SGRBG12_1X12, MIPI_CSI2_DT_RAW12 },
> + { MEDIA_BUS_FMT_SRGGB12_1X12, MIPI_CSI2_DT_RAW12 },
> + { MEDIA_BUS_FMT_Y12_1X12, MIPI_CSI2_DT_RAW12 },
> + { MEDIA_BUS_FMT_SBGGR14_1X14, MIPI_CSI2_DT_RAW14 },
> + { MEDIA_BUS_FMT_SGBRG14_1X14, MIPI_CSI2_DT_RAW14 },
> + { MEDIA_BUS_FMT_SGRBG14_1X14, MIPI_CSI2_DT_RAW14 },
> + { MEDIA_BUS_FMT_SRGGB14_1X14, MIPI_CSI2_DT_RAW14 },
> + { MEDIA_BUS_FMT_SBGGR16_1X16, MIPI_CSI2_DT_RAW16 },
> + { MEDIA_BUS_FMT_SGBRG16_1X16, MIPI_CSI2_DT_RAW16 },
> + { MEDIA_BUS_FMT_SGRBG16_1X16, MIPI_CSI2_DT_RAW16 },
> + { MEDIA_BUS_FMT_SRGGB16_1X16, MIPI_CSI2_DT_RAW16 },
> +};
> +
> +static const struct v4l2_mbus_framefmt formatter_default_fmt = {
> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
> + .width = 1920U,
> + .height = 1080U,
> + .field = V4L2_FIELD_NONE,
> + .colorspace = V4L2_COLORSPACE_SMPTE170M,
> + .xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
> + .ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
> + .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> +};
> +
> +static const struct csi_formatter_pix_format *csi_formatter_find_format(u32 code)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(formats); i++)
> + if (code == formats[i].code)
> + return &formats[i];
> +
> + return NULL;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * V4L2 subdev operations
> + */
> +
> +static inline struct csi_formatter *sd_to_formatter(struct v4l2_subdev *sdev)
> +{
> + return container_of(sdev, struct csi_formatter, sd);
> +}
> +
> +static int __csi_formatter_subdev_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_krouting *routing)
> +{
> + int ret;
> +
> + ret = v4l2_subdev_routing_validate(sd, routing,
> + V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
> + if (ret)
> + return ret;
> +
> + return v4l2_subdev_set_routing_with_fmt(sd, state, routing,
> + &formatter_default_fmt);
> +}
> +
> +static int csi_formatter_subdev_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state)
> +{
> + struct v4l2_subdev_route routes[] = {
> + {
> + .sink_pad = CSI_FORMATTER_PAD_SINK,
> + .sink_stream = 0,
> + .source_pad = CSI_FORMATTER_PAD_SOURCE,
> + .source_stream = 0,
> + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> + },
> + };
> +
> + struct v4l2_subdev_krouting routing = {
> + .num_routes = ARRAY_SIZE(routes),
> + .routes = routes,
> + };
> +
> + return __csi_formatter_subdev_set_routing(sd, sd_state, &routing);
> +}
> +
> +static int csi_formatter_subdev_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + if (code->pad == CSI_FORMATTER_PAD_SOURCE) {
> + struct v4l2_mbus_framefmt *fmt;
> +
> + if (code->index > 0)
> + return -EINVAL;
> +
> + fmt = v4l2_subdev_state_get_format(sd_state, code->pad,
> + code->stream);
> + code->code = fmt->code;
> + return 0;
> + }
> +
> + if (code->index >= ARRAY_SIZE(formats))
> + return -EINVAL;
> +
> + code->code = formats[code->index].code;
> +
> + return 0;
> +}
> +
> +static int csi_formatter_subdev_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *sdformat)
> +{
> + struct csi_formatter_pix_format const *format;
> + struct v4l2_mbus_framefmt *fmt;
> +
> + if (sdformat->pad == CSI_FORMATTER_PAD_SOURCE)
> + return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
> +
> + format = csi_formatter_find_format(sdformat->format.code);
> + if (!format)
> + format = &formats[0];
> +
> + v4l_bound_align_image(&sdformat->format.width, 1, 0xffff, 2,
> + &sdformat->format.height, 1, 0xffff, 0, 0);
> +
> + fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad,
> + sdformat->stream);
> + *fmt = sdformat->format;
> +
> + /* Set default code if user set an invalid value */
> + fmt->code = format->code;
> + sdformat->format = *fmt;
> +
> + /* Propagate the format from sink stream to source stream */
> + fmt = v4l2_subdev_state_get_opposite_stream_format(sd_state, sdformat->pad,
> + sdformat->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> + *fmt = sdformat->format;
> +
> + return 0;
> +}
> +
> +static int csi_formatter_subdev_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + enum v4l2_subdev_format_whence which,
> + struct v4l2_subdev_krouting *routing)
> +{
> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE &&
> + media_entity_is_streaming(&sd->entity))
> + return -EBUSY;
> +
> + return __csi_formatter_subdev_set_routing(sd, state, routing);
> +}
> +
> +static inline void csi_formatter_write(struct csi_formatter *formatter,
> + unsigned int reg, unsigned int value)
> +{
> + u32 offset = formatter->reg_offset;
> +
> + regmap_write(formatter->regs, reg + offset, value);
> +}
> +
> +static inline void csi_formatter_read(struct csi_formatter *formatter,
> + unsigned int reg, unsigned int *value)
> +{
> + u32 offset = formatter->reg_offset;
> +
> + regmap_read(formatter->regs, reg + offset, value);
> +}
> +
> +static u8 csi_formatter_get_index_by_dt(u8 data_type)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(formatter_dt_to_index_map); ++i) {
> + const struct csi_formatter_dt_index *entry =
> + &formatter_dt_to_index_map[i];
> +
> + if (data_type == entry->dtype)
> + return entry->index;
> + }
> +
> + pr_warn_once("Unsupported data type 0x%x, using default\n", data_type);
Why use the _once variant here? This doesn't look like a hot path, nor
does it seem like a condition we should silently ignore after the
first occurrence.
Also, if dev_warn_.. is not used here, pr_warn_once() should at least
give some context.
> +
> + return formatter_dt_to_index_map[0].index;
> +}
> +
> +static int csi_formatter_get_vc(struct csi_formatter *formatter,
> + struct v4l2_mbus_frame_desc *fd,
> + unsigned int stream)
> +{
> + struct v4l2_mbus_frame_desc_entry *entry = NULL;
> + unsigned int i;
> + u8 vc;
> +
> + for (i = 0; i < fd->num_entries; ++i) {
> + if (fd->entry[i].stream == stream) {
> + entry = &fd->entry[i];
> + break;
> + }
> + }
> +
> + if (!entry) {
> + dev_err(formatter->dev,
> + "No frame desc entry for stream %u\n", stream);
> + return -EPIPE;
> + }
> +
> + vc = entry->bus.csi2.vc;
> +
> + if (vc >= CSI_FORMATTER_VC_NUM) {
> + dev_err(formatter->dev, "Invalid virtual channel %u\n", vc);
> + return -EINVAL;
> + }
> +
> + return vc;
> +}
> +
> +static void csi_formatter_stop_stream(struct csi_formatter *formatter,
> + struct v4l2_subdev_state *state,
> + u64 stream_mask)
> +{
> + const struct csi_formatter_pix_format *pix_fmt;
> + struct v4l2_mbus_frame_desc fd = {};
> + struct v4l2_subdev_route *route;
> + struct v4l2_mbus_framefmt *fmt;
> + u32 val;
> + int vc;
> + int ret;
> +
> + ret = v4l2_subdev_call(formatter->csi_sd, pad, get_frame_desc,
> + formatter->remote_pad, &fd);
> + if (ret < 0 && ret != -ENOIOCTLCMD) {
> + dev_err(formatter->dev, "Failed to get frame desc: %d\n", ret);
> + return;
> + }
> +
> + for_each_active_route(&state->routing, route) {
> + if (route->source_pad != CSI_FORMATTER_PAD_SOURCE)
> + continue;
> +
> + if (!(stream_mask & BIT_ULL(route->source_stream)))
> + continue;
> +
> + if (ret == -ENOIOCTLCMD) {
> + /*
> + * Source doesn't implement get_frame_desc, use
> + * default VC 0
> + */
> + vc = 0;
> + } else {
> + vc = csi_formatter_get_vc(formatter, &fd, route->sink_stream);
> + if (vc < 0)
> + continue;
> + }
> +
> + fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> + route->sink_stream);
> +
> + pix_fmt = csi_formatter_find_format(fmt->code);
Do I understand correctly that csi_formatter_find_format() can't
return NULL at this point? If that's guaranteed by the surrounding
logic, it might be worth making that assumption explicit with a
WARN_ON or BUG_ON, to help catch future changes that could violate it.
> +
> + /* Clear only this stream's data type bit */
> + csi_formatter_read(formatter, CSI_VC_PIXEL_DATA_TYPE(vc), &val);
> + val &= ~BIT(csi_formatter_get_index_by_dt(pix_fmt->data_type));
> + csi_formatter_write(formatter, CSI_VC_PIXEL_DATA_TYPE(vc), val);
This read/modify/write pattern appears in several places. It might be
worth introducing a small helper around regmap_update_bits().
I haven't looked too deeply at the multi-stream handling, but could
you confirm that this doesn't break the case where two streams use the
same data type? As I understand it, when one stream is stopped, the
corresponding DT bit is cleared unconditionally, which could disable
the data type even though it is still required by another active
stream?
Regards,
Loic
> + }
> +}
> +
> +static int csi_formatter_start_stream(struct csi_formatter *formatter,
> + struct v4l2_subdev_state *state,
> + u64 stream_mask)
> +{
> + const struct csi_formatter_pix_format *pix_fmt;
> + struct v4l2_subdev_route *route;
> + struct v4l2_mbus_framefmt *fmt;
> + struct v4l2_mbus_frame_desc fd = {};
> + u64 configured_streams = 0;
> + u32 val;
> + int vc;
> + int ret;
> +
> + ret = v4l2_subdev_call(formatter->csi_sd, pad, get_frame_desc,
> + formatter->remote_pad, &fd);
> + if (ret < 0 && ret != -ENOIOCTLCMD) {
> + dev_err(formatter->dev, "Failed to get frame desc: %d\n", ret);
> + return ret;
> + }
> +
> + for_each_active_route(&state->routing, route) {
> + if (route->source_pad != CSI_FORMATTER_PAD_SOURCE)
> + continue;
> +
> + if (!(stream_mask & BIT_ULL(route->source_stream)))
> + continue;
> +
> + if (ret == -ENOIOCTLCMD) {
> + /*
> + * Source doesn't implement get_frame_desc, use
> + * default VC 0
> + */
> + vc = 0;
> + } else {
> + vc = csi_formatter_get_vc(formatter, &fd, route->sink_stream);
> + if (vc < 0) {
> + ret = vc;
> + goto err_cleanup;
> + }
> + }
> +
> + fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> + route->sink_stream);
> +
> + pix_fmt = csi_formatter_find_format(fmt->code);
> +
> + /* Update VC configuration */
> + csi_formatter_read(formatter, CSI_VC_PIXEL_DATA_TYPE(vc), &val);
> + val |= BIT(csi_formatter_get_index_by_dt(pix_fmt->data_type));
> + csi_formatter_write(formatter, CSI_VC_PIXEL_DATA_TYPE(vc), val);
> +
> + configured_streams |= BIT_ULL(route->source_stream);
> + }
> +
> + return 0;
> +
> +err_cleanup:
> + csi_formatter_stop_stream(formatter, state, configured_streams);
> + return ret;
> +}
> +
> +static int csi_formatter_subdev_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct csi_formatter *formatter = sd_to_formatter(sd);
> + struct device *dev = formatter->dev;
> + u64 sink_streams;
> + int ret;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state,
> + CSI_FORMATTER_PAD_SOURCE,
> + CSI_FORMATTER_PAD_SINK,
> + &streams_mask);
> + if (!sink_streams || !streams_mask)
> + return -EINVAL;
> +
> + guard(mutex)(&formatter->lock);
> +
> + if (!formatter->enabled_streams) {
> + ret = pm_runtime_resume_and_get(formatter->dev);
> + if (ret < 0) {
> + dev_err(dev, "Failed to resume runtime PM: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + ret = csi_formatter_start_stream(formatter, state, streams_mask);
> + if (ret)
> + goto err_runtime_put;
> +
> + ret = v4l2_subdev_enable_streams(formatter->csi_sd,
> + formatter->remote_pad,
> + sink_streams);
> + if (ret)
> + goto err_stop_stream;
> +
> + formatter->enabled_streams |= streams_mask;
> +
> + return 0;
> +
> +err_stop_stream:
> + csi_formatter_stop_stream(formatter, state, streams_mask);
> +err_runtime_put:
> + if (!formatter->enabled_streams)
> + pm_runtime_put(formatter->dev);
> + return ret;
> +}
> +
> +static int csi_formatter_subdev_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct csi_formatter *formatter = sd_to_formatter(sd);
> + u64 sink_streams;
> + int ret;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state,
> + CSI_FORMATTER_PAD_SOURCE,
> + CSI_FORMATTER_PAD_SINK,
> + &streams_mask);
> + if (!sink_streams || !streams_mask)
> + return -EINVAL;
> +
> + guard(mutex)(&formatter->lock);
> +
> + ret = v4l2_subdev_disable_streams(formatter->csi_sd, formatter->remote_pad,
> + sink_streams);
> + if (ret)
> + dev_err(formatter->dev, "Failed to disable streams: %d\n", ret);
> +
> + csi_formatter_stop_stream(formatter, state, streams_mask);
> +
> + formatter->enabled_streams &= ~streams_mask;
> +
> + if (!formatter->enabled_streams)
> + pm_runtime_put(formatter->dev);
> +
> + return ret;
> +}
> +
> +static const struct v4l2_subdev_pad_ops formatter_subdev_pad_ops = {
> + .enum_mbus_code = csi_formatter_subdev_enum_mbus_code,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .set_fmt = csi_formatter_subdev_set_fmt,
> + .get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
> + .set_routing = csi_formatter_subdev_set_routing,
> + .enable_streams = csi_formatter_subdev_enable_streams,
> + .disable_streams = csi_formatter_subdev_disable_streams,
> +};
> +
> +static const struct v4l2_subdev_ops formatter_subdev_ops = {
> + .pad = &formatter_subdev_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops formatter_internal_ops = {
> + .init_state = csi_formatter_subdev_init_state,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Media entity operations
> + */
> +
> +static const struct media_entity_operations formatter_entity_ops = {
> + .link_validate = v4l2_subdev_link_validate,
> + .get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
> +};
> +
> +static int csi_formatter_subdev_init(struct csi_formatter *formatter)
> +{
> + struct v4l2_subdev *sd = &formatter->sd;
> + int ret;
> +
> + v4l2_subdev_init(sd, &formatter_subdev_ops);
> +
> + snprintf(sd->name, sizeof(sd->name), "%s", dev_name(formatter->dev));
> + sd->internal_ops = &formatter_internal_ops;
> +
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> + V4L2_SUBDEV_FL_STREAMS;
> + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + sd->entity.ops = &formatter_entity_ops;
> + sd->dev = formatter->dev;
> +
> + formatter->pads[CSI_FORMATTER_PAD_SINK].flags = MEDIA_PAD_FL_SINK
> + | MEDIA_PAD_FL_MUST_CONNECT;
> + formatter->pads[CSI_FORMATTER_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&sd->entity, CSI_FORMATTER_PAD_NUM,
> + formatter->pads);
> + if (ret) {
> + dev_err(formatter->dev, "Failed to init pads\n");
> + return ret;
> + }
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + media_entity_cleanup(&sd->entity);
> +
> + return ret;
> +}
> +
> +static inline struct csi_formatter *
> +notifier_to_csi_formatter(struct v4l2_async_notifier *n)
> +{
> + return container_of(n, struct csi_formatter, notifier);
> +}
> +
> +static int csi_formatter_notify_bound(struct v4l2_async_notifier *notifier,
> + struct v4l2_subdev *sd,
> + struct v4l2_async_connection *asc)
> +{
> + const unsigned int link_flags = MEDIA_LNK_FL_IMMUTABLE
> + | MEDIA_LNK_FL_ENABLED;
> + struct csi_formatter *formatter = notifier_to_csi_formatter(notifier);
> + struct v4l2_subdev *sdev = &formatter->sd;
> + struct media_pad *sink = &sdev->entity.pads[CSI_FORMATTER_PAD_SINK];
> + struct media_pad *remote_pad;
> + int ret;
> +
> + formatter->csi_sd = sd;
> +
> + dev_dbg(formatter->dev, "Bound subdev: %s pad\n", sd->name);
> +
> + ret = v4l2_create_fwnode_links_to_pad(sd, sink, link_flags);
> + if (ret < 0)
> + return ret;
> +
> + remote_pad = media_pad_remote_pad_first(sink);
> + if (!remote_pad) {
> + dev_err(formatter->dev, "Pipe not setup correctly\n");
> + return -EPIPE;
> + }
> + formatter->remote_pad = remote_pad->index;
> +
> + return 0;
> +}
> +
> +static const struct v4l2_async_notifier_operations formatter_notify_ops = {
> + .bound = csi_formatter_notify_bound,
> +};
> +
> +static int csi_formatter_async_register(struct csi_formatter *formatter)
> +{
> + struct device *dev = formatter->dev;
> + struct v4l2_async_connection *asc;
> + int ret;
> +
> + struct fwnode_handle *ep __free(fwnode_handle) =
> + fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0,
> + FWNODE_GRAPH_ENDPOINT_NEXT);
> + if (!ep)
> + return -ENOTCONN;
> +
> + v4l2_async_subdev_nf_init(&formatter->notifier, &formatter->sd);
> +
> + asc = v4l2_async_nf_add_fwnode_remote(&formatter->notifier, ep,
> + struct v4l2_async_connection);
> + if (IS_ERR(asc)) {
> + ret = PTR_ERR(asc);
> + goto err_cleanup_notifier;
> + }
> +
> + formatter->notifier.ops = &formatter_notify_ops;
> +
> + ret = v4l2_async_nf_register(&formatter->notifier);
> + if (ret)
> + goto err_cleanup_notifier;
> +
> + ret = v4l2_async_register_subdev(&formatter->sd);
> + if (ret)
> + goto err_unregister_notifier;
> +
> + return 0;
> +
> +err_unregister_notifier:
> + v4l2_async_nf_unregister(&formatter->notifier);
> +err_cleanup_notifier:
> + v4l2_async_nf_cleanup(&formatter->notifier);
> + return ret;
> +}
> +
> +static void csi_formatter_async_unregister(struct csi_formatter *formatter)
> +{
> + v4l2_async_unregister_subdev(&formatter->sd);
> + v4l2_async_nf_unregister(&formatter->notifier);
> + v4l2_async_nf_cleanup(&formatter->notifier);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Suspend/resume
> + */
> +
> +static int csi_formatter_runtime_suspend(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct csi_formatter *formatter = sd_to_formatter(sd);
> +
> + clk_disable_unprepare(formatter->clk);
> +
> + return 0;
> +}
> +
> +static int csi_formatter_runtime_resume(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct csi_formatter *formatter = sd_to_formatter(sd);
> +
> + return clk_prepare_enable(formatter->clk);
> +}
> +
> +static DEFINE_RUNTIME_DEV_PM_OPS(csi_formatter_pm_ops,
> + csi_formatter_runtime_suspend,
> + csi_formatter_runtime_resume, NULL);
> +
> +static int csi_formatter_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct csi_formatter *formatter;
> + u32 val;
> + int ret;
> +
> + formatter = devm_kzalloc(dev, sizeof(*formatter), GFP_KERNEL);
> + if (!formatter)
> + return -ENOMEM;
> +
> + formatter->dev = dev;
> +
> + ret = devm_mutex_init(dev, &formatter->lock);
> + if (ret)
> + return ret;
> +
> + formatter->regs = syscon_node_to_regmap(dev->parent->of_node);
> + if (IS_ERR(formatter->regs))
> + return dev_err_probe(dev, PTR_ERR(formatter->regs),
> + "Failed to get csi formatter regmap\n");
> +
> + ret = of_property_read_u32(dev->of_node, "reg", &val);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "Failed to get csi formatter reg property\n");
> +
> + formatter->reg_offset = val;
> +
> + formatter->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(formatter->clk))
> + return dev_err_probe(dev, PTR_ERR(formatter->clk),
> + "Failed to get pixel clock\n");
> +
> + ret = csi_formatter_subdev_init(formatter);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to initialize formatter subdev\n");
> +
> + platform_set_drvdata(pdev, &formatter->sd);
> +
> + /* Enable runtime PM. */
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + goto err_cleanup_subdev;
> +
> + ret = csi_formatter_async_register(formatter);
> + if (ret < 0) {
> + dev_err_probe(dev, ret, "Failed to register async subdevice\n");
> + goto err_cleanup_subdev;
> + }
> +
> + return 0;
> +
> +err_cleanup_subdev:
> + v4l2_subdev_cleanup(&formatter->sd);
> + media_entity_cleanup(&formatter->sd.entity);
> + return ret;
> +}
> +
> +static void csi_formatter_remove(struct platform_device *pdev)
> +{
> + struct v4l2_subdev *sd = platform_get_drvdata(pdev);
> + struct csi_formatter *formatter = sd_to_formatter(sd);
> +
> + csi_formatter_async_unregister(formatter);
> +
> + v4l2_subdev_cleanup(&formatter->sd);
> + media_entity_cleanup(&formatter->sd.entity);
> +}
> +
> +static const struct of_device_id csi_formatter_of_match[] = {
> + { .compatible = "fsl,imx95-csi-formatter" },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, csi_formatter_of_match);
> +
> +static struct platform_driver csi_formatter_device_driver = {
> + .driver = {
> + .name = "csi-pixel-formatter",
> + .of_match_table = csi_formatter_of_match,
> + .pm = pm_ptr(&csi_formatter_pm_ops),
> + },
> + .probe = csi_formatter_probe,
> + .remove = csi_formatter_remove,
> +};
> +
> +module_platform_driver(csi_formatter_device_driver);
> +
> +MODULE_AUTHOR("NXP Semiconductor, Inc.");
> +MODULE_DESCRIPTION("NXP i.MX95 CSI Pixel Formatter driver");
> +MODULE_LICENSE("GPL");
>
> --
> 2.34.1
>
>
^ permalink raw reply
* Re: [PATCH v2 0/2] hwrng: Enable EIP-76 on RZ/N1
From: Herbert Xu @ 2026-07-17 8:17 UTC (permalink / raw)
To: Miquel Raynal (Schneider Electric)
Cc: Olivia Mackall, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jayesh Choudhary, Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang,
linux-crypto, devicetree, linux-kernel
In-Reply-To: <20260710-schneider-v7-2-rc1-eip76-upstream-v2-0-4eab557b0e70@bootlin.com>
On Fri, Jul 10, 2026 at 08:20:31PM +0200, Miquel Raynal (Schneider Electric) wrote:
> The EIP-76 is an block from Inside-Secure which is not at all OMAP
> specific. Make it available on Renesas RZ/N1 where it is present inside
> a container named EIP-150.
>
> These patches come from a bigger series which I split into meaningful
> patchsets, one per subsystem:
>
> Link: https://lore.kernel.org/all/20260327-schneider-v7-0-rc1-crypto-v1-0-5e6ff7853994@bootlin.com/
>
> Changes in v2:
> - Patches have been extracted from the bigger series.
> - Rebased on top of v7.2-rc1.
> - Tag from Rob collected.
>
> Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
> ---
> Miquel Raynal (Schneider Electric) (2):
> dt-bindings: rng: Rename the title of the EIP-76 file
> hwrng: omap: Enable on Renesas RZ/N1D
>
> Documentation/devicetree/bindings/rng/inside-secure,safexcel-eip76.yaml | 2 +-
> drivers/char/hw_random/Kconfig | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
> ---
> base-commit: 7f26e010764df304602f33912a0550dcf46e72c2
> change-id: 20260710-schneider-v7-2-rc1-eip76-upstream-4d6a83015f7e
All applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v7 02/13] dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller
From: Krzysztof Kozlowski @ 2026-07-17 8:15 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, Vinod Koul,
Neil Armstrong, Russell King, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, linux-phy
In-Reply-To: <20260717-berserk-faithful-cat-a34ad8@quoll>
On 17/07/2026 08:36, Krzysztof Kozlowski wrote:
> On Fri, Jul 17, 2026 at 12:35:38AM +0300, Stefan Dösinger wrote:
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index cb50c2e3b4ea..5f962b9947ab 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3878,10 +3878,13 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>> S: Odd fixes
>> F: Documentation/arch/arm/zte/
>> F: Documentation/devicetree/bindings/arm/zte.yaml
>> +F: Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml
>> F: Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.yaml
>> F: arch/arm/boot/dts/zte/
>> F: arch/arm/mach-zte/
>> +F: include/dt-bindings/clock/zte,zx297520v3-clk.h
>> F: include/dt-bindings/phy/phy-zte-zx297520v3-usb.h
>> +F: include/dt-bindings/reset/zte,zx297520v3-reset.h
>>
>> ARM/ZYNQ ARCHITECTURE
>> M: Michal Simek <michal.simek@amd.com>
>> diff --git a/include/dt-bindings/clock/zte,zx297520v3-clk.h b/include/dt-bindings/clock/zte,zx297520v3-clk.h
>
> Same mistake. Filename MUST match the compatible.
I did not notice that you combine here all the clock IDs from other
clock controllers. The header filename is therefore fine (also reset fine).
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v8 2/8] iio: dac: ad5686: missing NULL check on match data
From: Andy Shevchenko @ 2026-07-17 8:15 UTC (permalink / raw)
To: Rodrigo Alencar
Cc: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Kees Cook, Gustavo A. R. Silva,
sashiko-bot
In-Reply-To: <ondynh4kmchzugk2l24klamrcaje3qxnhgiks7gnc46wwwvo35@hysk2zaimb2e>
On Fri, Jul 17, 2026 at 07:51:35AM +0100, Rodrigo Alencar wrote:
> On 16/07/26 21:42, Andy Shevchenko wrote:
> > On Thu, Jul 16, 2026 at 01:14:18PM +0100, Rodrigo Alencar via B4 Relay wrote:
...
> > > struct iio_dev *indio_dev;
> > > int ret, i;
> >
> > Move an assignment here as well.
> >
> > chip_info = ...
>
> assignment? this is an input param validation
Can we make sure we always get a correct one to begin with?
With that an assignment and check can be coupled together.
Also consider use -ENODATA as it's most likely comes from
driver_data.
> > > + if (!chip_info)
> > > + return -ENODEV;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 0/3] Add AD5710R/AD5711R DAC
From: Andy Shevchenko @ 2026-07-17 8:10 UTC (permalink / raw)
To: Kim Seer Paller
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, linux-iio, linux-kernel, linux, devicetree
In-Reply-To: <DK0OK76VQ1QV.3U5B7UFI0UB9F@analog>
On Fri, Jul 17, 2026 at 03:50:08PM +0800, Kim Seer Paller wrote:
> On Fri Jul 17, 2026 at 1:58 AM PST, Andy Shevchenko wrote:
> > On Thu, Jul 16, 2026 at 03:59:24PM +0800, Kim Seer Paller wrote:
> > > The AD5710R (16-bit) and AD5711R (12-bit) are 8-channel, low-power,
> > > configurable current/voltage output DACs with an on-chip 2.5V reference.
> > > Each channel can be independently configured as a voltage output
> > > (0V to VREF or 0V to 2xVREF) or a current output (0mA to 50mA).
> > >
> > > This series adds support for these parts:
> > > - ABI documentation for the high_z powerdown mode and the current-output
> > > powerdown attributes
> > > - device tree bindings for the AD5710R/AD5711R
> > > - basic IIO DAC driver, supporting per-channel voltage/current output
> > > selection, per-channel powerdown, software or hardware (LDAC) DAC
> > > updates, optional output range doubling, internal or external
> > > reference, and hardware reset (with a software-reset fallback).
> >
> > Why a brand new driver?
>
> After looking more closely, the register map is the same as the AD3530R family.
> I originally kept it separate because of the functional differences: per channel
> VDAC/IDAC selection described in device tree via adi,ch-func, selectable 12/16 bit
> resolution, and current output path. But those can be fit the existing chip_info
> abstraction. Given that, I'm leaning toward extending ad3530r to support the
> ad5710r/ad5711r and dropping the standalone driver. Does that direction work for
> you? If so, I will do it in v2.
Yes, pretty much!
> > > Datasheet:
> > > https://www.analog.com/media/en/technical-documentation/data-sheets/ad5711r-ad5710r.pdf
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 06/10] dt-bindings: power: renesas,r8a78000-mdlc: Add firmware property
From: Sudeep Holla @ 2026-07-17 8:07 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Cristian Marussi, Rob Herring, Sudeep Holla, Krzysztof Kozlowski,
Conor Dooley, Saravana Kannan, Ulf Hansson, Rafael J . Wysocki,
Philipp Zabel, Michael Turquette, Stephen Boyd, Brian Masney,
Bartosz Golaszewski, Vinod Koul, Konrad Dybcio, Kevin Hilman,
Florian Fainelli, Wolfram Sang, Marek Vasut, Kuninori Morimoto,
arm-scmi, linux-arm-kernel, devicetree, linux-pm, linux-clk,
linux-renesas-soc, linux-kernel
In-Reply-To: <b4d16ddb0c0af56f4103feadb3014e71899edec1.1783505329.git.geert+renesas@glider.be>
On Wed, Jul 08, 2026 at 12:16:24PM +0200, Geert Uytterhoeven wrote:
> Add a firmware property, for systems where SCMI must be used instead of
> direct hardware access.
>
Please add more info to explain why you think this is reasonable
approach. Until then ...
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
> - Spin off from "dt-bindings: power: Document Renesas R-Car X5H Module
> Controller".
> ---
> .../devicetree/bindings/power/renesas,r8a78000-mdlc.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/renesas,r8a78000-mdlc.yaml b/Documentation/devicetree/bindings/power/renesas,r8a78000-mdlc.yaml
> index 1dba1e1b1c5b52d0..9616f0244415896c 100644
> --- a/Documentation/devicetree/bindings/power/renesas,r8a78000-mdlc.yaml
> +++ b/Documentation/devicetree/bindings/power/renesas,r8a78000-mdlc.yaml
> @@ -43,6 +43,12 @@ properties:
> and thus identical to the second power domain specifier cell.
> const: 1
>
> + firmware:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Reference to the SCMI firmware device node on systems where SCMI must be
> + used instead of direct hardware access.
> +
NACK
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH v2 00/10] R-Car X5H Ironhide CPG/MDLC SCMI remapping support
From: Sudeep Holla @ 2026-07-17 8:05 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Cristian Marussi, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Sudeep Holla, Saravana Kannan, Ulf Hansson, Rafael J . Wysocki,
Philipp Zabel, Michael Turquette, Stephen Boyd, Brian Masney,
Bartosz Golaszewski, Vinod Koul, Konrad Dybcio, Kevin Hilman,
Florian Fainelli, Wolfram Sang, Marek Vasut, Kuninori Morimoto,
arm-scmi, linux-arm-kernel, devicetree, linux-pm, linux-clk,
linux-renesas-soc, linux-kernel
In-Reply-To: <cover.1783505329.git.geert+renesas@glider.be>
On Wed, Jul 08, 2026 at 12:16:18PM +0200, Geert Uytterhoeven wrote:
> Hi all,
>
> TL;DR:
>
> Describe hardware in DT, and perform the mapping to SCMI in Linux.
>
> The Renesas R-Car X5H-based Ironhide board is the first Renesas
> SoC/board combination that implements the ARM System Control and
> Management Interface (SCMI).
>
> This means Linux can no longer perform various system operations (e.g.
> clock, power domain, and reset control) by accessing the hardware
> directly. Instead, these operations are abstracted according to various
> SCMI sub-protocols, and Linux has to send messages to an SCMI-compliant
> firmware running on a System Control Processor (SCP).
> More specifically, the R-Car X5H SCP FW SCMI controls access to:
> 1. Core clocks and module clocks,
> 2. Module resets,
> 3. Power domains,
> 4. System power.
> The latter is not relevant for this discussion.
>
> Using SCMI also has an impact on the board DTS: besides the addition of
> a firmware/scmi node with protocol-specific subnodes, devices using
> SCMI-abstracted system resources now have to refer to these resources
> using resource specifiers containing phandles to the SCMI protocol
> subnodes, and firmware-specific object IDs[1].
>
> Unfortunately we encountered several issues with this:
> - The clock, reset, and power domain IDs are not stable by design,
> causing DT incompatibilities.
> SCMI requires that the ID space exposed to an agent (i.e. Linux)
> starts at zero and is contiguous. Hence any change in system
> partitioning (Application CPU core vs. Realtime CPU core,
> virtualization, ...) may mean a renumbering of all SCMI IDs,
> implying a massive change to the DT.
Yes it is well known fact. But where this is a feature or limitation
is debatable. Why would SCP firmware churn and re-number or re-assign
a differ domain ID everytime it is updated is questionable to me.
> - There may be different SCP firmware lineages, e.g. proprietary and
> certified variants, and FLOSS variants,
No sure if I understand the issue here.
> - SCMI features may differ across firmware versions,
Again, features must be discoverable that is the advantage of DT vs this
firmware interface, so not sure what is the issue in this matter.
> - SCMI may abstract the hardware in a different way, requiring changes
> in (possibly existing) DT bindings and drivers to support the same
> hardware with and without SCMI:
> - E.g. the number of SCMI clocks inputs to a hardware block may
> differ from the actual hardware,
> - E.g. hardware clock inputs may be abstracted as SCMI power
> domains (or vice versa), like on Qualcomm SA8255P[2].
True, the moment SCMI like interface is used in OSPM, the firmware
description hides real h/w details and provide what firmware thinks
OSPM must view hardware as(how VMMs virtualise things for VMs, firmware
can have similar effect with such interfaces).
> Experience has taught that not describing the hardware in DT may
> lead to major issues later.
OK, that's interesting. But the whole point with these interfaces is to
hide those, so just don't use such abstraction interface if you want the
hardware to be described as is to the OSPM.
> - SCMI does not support the concept of a clock domain, so existing
> drivers can no longer rely on Runtime PM, but need to be changed to
> manage functional clocks explicitly,
Not sure what that means, you mean clock protocol is not good enough to
model the h/w clock domains on your platform ? If so, you are welcome to
improve the specification.
> - U-Boot IPL (Initial Program Loader) needs a proper hardware
> description in DT, not using SCMI.
>
I am going to ignore that.
> This list deliberately ignores ID instability and the need for SCMI
> quirks during development of the SCMI firmware. While upstream Linux
> must only support sufficiently mature firmware versions, vendor trees
> (e.g. renesas-lts) may still want to support development firmware
> versions.
>
Fair enough, but the acceptance of such changes depends on maintenance
and possible absuse with such spec deviations.
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 3/3] spi: amlogic: spisg: Add support for A9 controller features
From: sashiko-bot @ 2026-07-17 8:04 UTC (permalink / raw)
To: Xianwei Zhao; +Cc: conor+dt, devicetree, neil.armstrong, linux-amlogic, robh
In-Reply-To: <20260717-a9-spisg-v1-3-2eeea77be19f@amlogic.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The calculated CFG_CS_HOLD delay value lacks bounds checking before being written via FIELD_MODIFY, leading to silent bitwise truncation and incorrectly shortened hold times.
Pre-existing issues:
- [High] The driver fails to abort the hardware operation when a transfer times out, leading to DMA use-after-free and cascading synchronization failures.
- [High] `spi_delay_to_sclk()` suffers from a 32-bit integer overflow when calculating delay cycles, resulting in drastically shorter setup and hold times than requested.
--
--- Patch [3]: [PATCH 3/3] spi: amlogic: spisg: Add support for A9 controller features ---
commit 850f7cb5955f8bd514dfd78b459ee02b15378db4
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
spi: amlogic: spisg: Add support for A9 controller features
The Amlogic A9 SPISG controller extends the A4 controller with additional
configuration options. Add SoC-specific capability data and configure
these features when supported by the hardware, while keeping compatibility
with existing A4 controllers.
> diff --git a/drivers/spi/spi-amlogic-spisg.c b/drivers/spi/spi-amlogic-spisg.c
> index 0f026d3e43e0b..845eb81d483a3 100644
> --- a/drivers/spi/spi-amlogic-spisg.c
> +++ b/drivers/spi/spi-amlogic-spisg.c
[ ... ]
> @@ -525,9 +541,17 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
> }
>
> /* calculate cs-setup delay with the first xfer speed */
> - if (list_is_first(&xfer->transfer_list, &msg->transfers))
> - desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP,
> - spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_setup));
> + if (list_is_first(&xfer->transfer_list, &msg->transfers)) {
> + val = spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_setup);
[Severity: High]
This is a pre-existing issue, but can spi_delay_to_sclk() suffer from a
32-bit integer overflow when calculating delay cycles?
Looking at spi_delay_to_sclk():
drivers/spi/spi-amlogic-spisg.c:spi_delay_to_sclk() {
...
return DIV_ROUND_UP_ULL(slck_speed_hz * ns, NSEC_PER_SEC);
}
If a client requests a large delay, the multiplication of the two 32-bit
integers (slck_speed_hz and ns) can wrap around before being cast to a
64-bit integer by DIV_ROUND_UP_ULL.
Could this result in drastically shorter setup and hold times than
requested?
> + if (data && data->cs_setup_extend_ctrl) {
> + val = min_t(u32, 0xFF, val);
> + desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP, val & 0xF);
> + FIELD_MODIFY(CFG_CS_SETUP_EXTEND, &spisg->cfg_spi, val >> 4);
> + } else {
> + val = min_t(u32, 0xF, val);
> + desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP, val);
> + }
> + }
>
> /* calculate cs-hold delay with the last xfer speed */
> if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
> @@ -542,7 +566,12 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
> xfer->effective_speed_hz);
> }
>
> - if (cs_hold_in_sclk) {
> + if (data && data->cs_hold_ctrl) {
> + cs_hold_in_sclk = cs_hold_in_sclk ? : 1;
> + val = cs_hold_in_sclk * (FIELD_GET(CFG_CLK_DIV, spisg->cfg_bus) + 1);
> + FIELD_MODIFY(CFG_CS_HOLD, &spisg->cfg_spi, val);
[Severity: High]
Does the calculated CFG_CS_HOLD delay value lack bounds checking before
being written via FIELD_MODIFY?
Unlike CFG_CS_SETUP which safely uses min_t(), cs_hold is left unbounded.
Since CFG_CS_HOLD is a 15-bit field, if an SPI client requests a large CS
hold time, the FIELD_MODIFY macro will silently apply a bitwise mask.
Will this cause the delay to wrap around (e.g. 32768 becomes 0) rather
than clamping at the maximum supported hardware limit, leading to hardware
timing violations?
[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the
timeout error path to free DMA-mapped memory without halting the active
hardware?
Looking further down in aml_spisg_transfer_one_message():
drivers/spi/spi-amlogic-spisg.c:aml_spisg_transfer_one_message() {
...
if (wait_for_completion_timeout(&spisg->completion, ...))
ret = spisg->status ? -EIO : 0;
else
ret = -ETIMEDOUT;
dma_unmap_single(dev, descs_paddr, descs_len, DMA_TO_DEVICE);
end:
...
kfree(descs);
...
}
If an SPI transfer times out and wait_for_completion_timeout() expires,
the code proceeds directly to unmap the DMA memory and free the descriptor
array. However, the hardware is never halted or reset (e.g. by writing 0
to the descriptor list register).
Might the hardware still be actively performing DMA, resulting in memory
corruption from a use-after-free? Furthermore, if the hardware eventually
completes, could the delayed interrupt falsely wake up subsequent transfers?
> + desc--;
> + } else if (cs_hold_in_sclk) {
> /* additional null-descriptor to achieve the cs-hold delay */
> aml_spisg_setup_null_desc(spisg, desc, cs_hold_in_sclk);
> desc--;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-a9-spisg-v1-0-2eeea77be19f@amlogic.com?part=3
^ permalink raw reply
* Re: [PATCH 2/3] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor
From: sashiko-bot @ 2026-07-17 8:01 UTC (permalink / raw)
To: Xianwei Zhao; +Cc: linux-amlogic, devicetree, conor+dt, neil.armstrong, robh
In-Reply-To: <20260717-a9-spisg-v1-2-2eeea77be19f@amlogic.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing hardware abort on SPI transfer timeout leads to DMA Use-After-Free and memory corruption.
- [High] 32-bit integer overflow in `spi_delay_to_sclk` leads to severely truncated SPI delays.
- [High] `SPISG_BLOCK_MAX` exceeds the hardware mask size, causing maximum-length transfers to be processed as zero-length.
- [High] Hardware limits for `cs_setup` and `cs_hold` delays are not clamped before being packed into register fields, causing long delays to silently wrap around to short values.
--
--- Patch [2]: [PATCH 2/3] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor ---
commit 07f3c7893cd8bdd39413002b5f617a22e93e0842
Author: Sunny Luo <sunny.luo@amlogic.com>
spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor
This patch fixes an issue where the CFG_KEEP_SS flag is unconditionally
cleared on the last SPI descriptor. It records the cs_change value of
the last transfer and properly programs CFG_KEEP_SS on the final descriptor.
When a null descriptor is used for cs-hold, the flag is kept on the
preceding descriptor and the recorded value is applied at the end.
> diff --git a/drivers/spi/spi-amlogic-spisg.c b/drivers/spi/spi-amlogic-spisg.c
> index afc8af04638d3..0f026d3e43e0b 100644
> --- a/drivers/spi/spi-amlogic-spisg.c
> +++ b/drivers/spi/spi-amlogic-spisg.c
> @@ -529,9 +530,11 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
> spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_setup));
[Severity: High]
This isn't a bug introduced by this patch, but can the 32-bit multiplication
in spi_delay_to_sclk() overflow here?
If a requested delay in nanoseconds and the SPI clock frequency in Hz are
multiplied together, it evaluates as a 32-bit unsigned operation before being
cast to 64-bit for the division:
drivers/spi/spi-amlogic-spisg.c:spi_delay_to_sclk() {
...
return DIV_ROUND_UP_ULL(slck_speed_hz * ns, NSEC_PER_SEC);
}
For example, at a 100MHz clock, a delay greater than 42.9us will wrap around,
causing hardware setup or hold delays to become extremely short or zero.
[Severity: High]
This is also a pre-existing issue, but are the hardware limits for cs_setup
and cs_hold delays clamped before being packed into the register fields?
CFG_CS_SETUP is a 4-bit field (max 15). If spi_delay_to_sclk() returns
a value greater than 15, FIELD_PREP() will silently truncate the upper
bits, resulting in an incorrect, drastically shorter delay.
> /* calculate cs-hold delay with the last xfer speed */
> - if (list_is_last(&xfer->transfer_list, &msg->transfers))
> + if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
> cs_hold_in_sclk =
> spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_hold);
> + last_xfer_keep_ss = xfer->cs_change;
> + }
[ ... ]
[Severity: Critical]
This is a pre-existing issue, but does the error handling path for timeouts in
aml_spisg_transfer_one_message() lack a hardware abort?
If a hardware transfer times out, the driver unmaps DMA buffers and frees the
transfer descriptors without stopping the SPI controller hardware:
drivers/spi/spi-amlogic-spisg.c:aml_spisg_transfer_one_message() {
...
if (wait_for_completion_timeout(&spisg->completion, ...))
ret = spisg->status ? -EIO : 0;
else
ret = -ETIMEDOUT;
dma_unmap_single(dev, descs_paddr, descs_len, DMA_TO_DEVICE);
end:
...
}
Could this lead to the hardware continuing to write to or read from freed memory
pages (DMA Use-After-Free) since the hardware isn't stopped?
[Severity: High]
This is a pre-existing issue, but can the maximum blocks count exceed the
hardware mask size in aml_spisg_setup_transfer()?
The blocks count is capped at SPISG_BLOCK_MAX (0x100000):
drivers/spi/spi-amlogic-spisg.c:aml_spisg_setup_transfer() {
...
blocks = min_t(int, blocks, SPISG_BLOCK_MAX);
desc->cfg_start |= FIELD_PREP(CFG_BLOCK_SIZE, block_size & 0x7) |
FIELD_PREP(CFG_BLOCK_NUM, blocks);
...
}
Since CFG_BLOCK_NUM uses a 20-bit mask (GENMASK(19, 0)), a transfer bounded
exactly at SPISG_BLOCK_MAX (0x100000) will be silently zeroed out when masked,
causing it to be processed as a zero-length transfer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-a9-spisg-v1-0-2eeea77be19f@amlogic.com?part=2
^ permalink raw reply
* Re: [PATCH 1/3] iio: ABI: add DAC high_z powerdown mode and current output entries
From: Kim Seer Paller @ 2026-07-17 7:58 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, linux-iio, linux-kernel, linux, devicetree
In-Reply-To: <alkcBaHV0gHY37wR@ashevche-desk.local>
On Fri Jul 17, 2026 at 1:59 AM PST, Andy Shevchenko wrote:
> On Thu, Jul 16, 2026 at 03:59:25PM +0800, Kim Seer Paller wrote:
> > Add high_z powerdown mode for DACs with high impedance output in
> > current mode (IDAC) and 15kohm_to_gnd resistor to GND. Also add
> > out_currentY_powerdown_mode, out_currentY_powerdown_mode_available,
> > and out_currentY_powerdown entries to document current output
> > powerdown support.
>
> ...
>
> > 125kohm_to_gnd: connected to ground via an 125kOhm resistor,
> > 500kohm_to_gnd: connected to ground via a 500kOhm resistor,
> > 640kohm_to_gnd: connected to ground via a 640kOhm resistor,
> > - three_state: left floating.
> > + three_state: left floating,
> > + high_z: left floating.
>
> It may be less churn if just located before three_state, but hey, why?!
> three_state is a synonym to high_z. What's the impediment to use it?
You are right, thanks. I added high_z because that is the term the datasheet
uses for current output powerdown mode. Since three_state means the same
thing I will drop the high_z addition and use the existing three_state.
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 2/8] dt-bindings: gpio: cdns: add Axiado AX3005 GPIO variant
From: Swark Yang @ 2026-07-17 7:57 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-i3c, Alexandre Belloni, robh, Frank.Li, conor+dt,
devicetree
In-Reply-To: <20260717040056.6BA971F000E9@smtp.kernel.org>
On 7/17/2026 12:00 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The schema enforces a 3-item compatible string chain that unintentionally forces AX3005 devices to inherit an AX3000-specific driver quirk, contradicting the commit message.
> --
Thanks for the report.
To clarify, the 3-item fallback in the code is intentional and correct.
The AX3005 GPIO block is fully compatible with the AX3000 implementation
and indeed requires the exact same driver quirks (such as the specific
direction mode and output enable handling).
The issue is just a poor description in my commit message. It should
have been worded as follows:
"Add binding for the Axiado AX3005 GPIO controller. The AX3005 GPIO
block is fully compatible with the AX3000 implementation and requires
the same driver quirks. Therefore, it is added with a fallback to the
'axiado,ax3000-gpio' compatible string, which in turn falls back to the
generic 'cdns,gpio-r1p02'."
I will hold off on sending a v4 just for this commit message update to
avoid unnecessary noise on the list. I'll wait for further review
comments on the rest of the series and will include this updated message
if a v4 re-roll is needed.
Best Regards,
Swark
^ 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