* Re: [PATCH v1 3/5] dt-bindings: gpio: mpfs: allow gpio-line-names
From: Rob Herring @ 2024-03-27 17:57 UTC (permalink / raw)
To: Conor Dooley
Cc: Lorenzo Pieralisi, Valentina Fernandez, Bjorn Helgaas,
Linus Walleij, Bartosz Golaszewski, Daire McNamara, linux-riscv,
linux-kernel, linux-pci, devicetree, Conor Dooley,
Krzysztof Kozlowski, linux-gpio, Jamie Gibbons,
Krzysztof Wilczyński
In-Reply-To: <20240327-overrate-overuse-1e32abccd001@spud>
On Wed, 27 Mar 2024 12:24:38 +0000, Conor Dooley wrote:
> From: Jamie Gibbons <jamie.gibbons@microchip.com>
>
> The BeagleV Fire devicetree will make use of gpio-line-names, allow it
> in the binding.
>
> Signed-off-by: Jamie Gibbons <jamie.gibbons@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v1 2/5] dt-bindings: gpio: mpfs: add coreGPIO support
From: Rob Herring @ 2024-03-27 17:57 UTC (permalink / raw)
To: Conor Dooley
Cc: Jamie Gibbons, Bartosz Golaszewski, linux-kernel,
Lorenzo Pieralisi, Daire McNamara, devicetree,
Krzysztof Kozlowski, linux-pci, Linus Walleij, Conor Dooley,
Krzysztof Wilczyński, linux-gpio, Valentina Fernandez,
linux-riscv, Bjorn Helgaas
In-Reply-To: <20240327-procurer-rascal-33bca7d5d14b@spud>
On Wed, 27 Mar 2024 12:24:37 +0000, Conor Dooley wrote:
> From: Jamie Gibbons <jamie.gibbons@microchip.com>
>
> The GPIO controllers on PolarFire SoC were based on the "soft" IP
> CoreGPIO, but the inp/outp registers are at different offsets. Add
> compatible to allow for support of both sets of offsets. The soft
> core will not always have interrupts wired up, so only enforce them for
> the "hard" core on PolarFire SoC.
>
> Signed-off-by: Jamie Gibbons <jamie.gibbons@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../bindings/gpio/microchip,mpfs-gpio.yaml | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v1 1/5] dt-bindings: riscv: microchip: document beaglev-fire
From: Rob Herring @ 2024-03-27 17:56 UTC (permalink / raw)
To: Conor Dooley
Cc: Bartosz Golaszewski, Linus Walleij, Lorenzo Pieralisi,
Bjorn Helgaas, linux-pci, Daire McNamara, linux-riscv,
Conor Dooley, Jamie Gibbons, Krzysztof Wilczyński,
linux-kernel, linux-gpio, devicetree, Valentina Fernandez,
Krzysztof Kozlowski
In-Reply-To: <20240327-camper-brownnose-6392076d8699@spud>
On Wed, 27 Mar 2024 12:24:36 +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> The BeagleV Fire is a BeagleBone Black form-factor board with a
> PolarFire SoC.
>
> Link: https://www.beagleboard.org/boards/beaglev-fire
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Documentation/devicetree/bindings/riscv/microchip.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: rockchip: Add VEPU121 to rk3588
From: Sebastian Reichel @ 2024-03-27 17:45 UTC (permalink / raw)
To: Emmanuel Gil Peyrot
Cc: linux-kernel, Ezequiel Garcia, Philipp Zabel,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
Robin Murphy, Cristian Ciocaltea, Dragan Simic, Shreeya Patel,
Chris Morgan, Andy Yan, Nicolas Frattaroli, linux-media,
linux-rockchip, devicetree, linux-arm-kernel, iommu
In-Reply-To: <20240327134115.424846-3-linkmauve@linkmauve.fr>
[-- Attachment #1: Type: text/plain, Size: 4157 bytes --]
Hi,
On Wed, Mar 27, 2024 at 02:41:12PM +0100, Emmanuel Gil Peyrot wrote:
> The TRM (version 1.0 page 385) lists five VEPU121 cores, but only four
> interrupts are listed (on page 24), so I’ve only enabled four of them
> for now.
>
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 80 +++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index 87b83c87bd55..510ed3db9d01 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -2488,6 +2488,86 @@ gpio4: gpio@fec50000 {
> };
> };
>
> + jpeg_enc0: video-codec@fdba0000 {
> + compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";
> + reg = <0x0 0xfdba0000 0x0 0x800>;
> + interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_JPEG_ENCODER0>, <&cru HCLK_JPEG_ENCODER0>;
> + clock-names = "aclk", "hclk";
> + iommus = <&jpeg_enc0_mmu>;
> + power-domains = <&power RK3588_PD_VDPU>;
> + };
> +
> + jpeg_enc0_mmu: iommu@fdba0800 {
> + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> + reg = <0x0 0xfdba0800 0x0 0x40>;
> + interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_JPEG_ENCODER0>, <&cru HCLK_JPEG_ENCODER0>;
> + clock-names = "aclk", "iface";
> + power-domains = <&power RK3588_PD_VDPU>;
> + #iommu-cells = <0>;
> + };
> +
> + jpeg_enc1: video-codec@fdba4000 {
> + compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";
> + reg = <0x0 0xfdba4000 0x0 0x800>;
> + interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_JPEG_ENCODER1>, <&cru HCLK_JPEG_ENCODER1>;
> + clock-names = "aclk", "hclk";
> + iommus = <&jpeg_enc1_mmu>;
> + power-domains = <&power RK3588_PD_VDPU>;
> + };
> +
> + jpeg_enc1_mmu: iommu@fdba4800 {
> + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> + reg = <0x0 0xfdba4800 0x0 0x40>;
> + interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_JPEG_ENCODER1>, <&cru HCLK_JPEG_ENCODER1>;
> + clock-names = "aclk", "iface";
> + power-domains = <&power RK3588_PD_VDPU>;
> + #iommu-cells = <0>;
> + };
> +
> + jpeg_enc2: video-codec@fdba8000 {
> + compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";
> + reg = <0x0 0xfdba8000 0x0 0x800>;
> + interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_JPEG_ENCODER2>, <&cru HCLK_JPEG_ENCODER2>;
> + clock-names = "aclk", "hclk";
> + iommus = <&jpeg_enc2_mmu>;
> + power-domains = <&power RK3588_PD_VDPU>;
> + };
> +
> + jpeg_enc2_mmu: iommu@fdba8800 {
> + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> + reg = <0x0 0xfdba8800 0x0 0x40>;
> + interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_JPEG_ENCODER2>, <&cru HCLK_JPEG_ENCODER2>;
> + clock-names = "aclk", "iface";
> + power-domains = <&power RK3588_PD_VDPU>;
> + #iommu-cells = <0>;
> + };
> +
> + jpeg_enc3: video-codec@fdbac000 {
> + compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";
> + reg = <0x0 0xfdbac000 0x0 0x800>;
> + interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_JPEG_ENCODER3>, <&cru HCLK_JPEG_ENCODER3>;
> + clock-names = "aclk", "hclk";
> + iommus = <&jpeg_enc3_mmu>;
> + power-domains = <&power RK3588_PD_VDPU>;
> + };
> +
> + jpeg_enc3_mmu: iommu@fdbac800 {
> + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> + reg = <0x0 0xfdbac800 0x0 0x40>;
> + interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_JPEG_ENCODER3>, <&cru HCLK_JPEG_ENCODER3>;
> + clock-names = "aclk", "iface";
> + power-domains = <&power RK3588_PD_VDPU>;
> + #iommu-cells = <0>;
> + };
> +
> av1d: video-codec@fdc70000 {
> compatible = "rockchip,rk3588-av1-vpu";
> reg = <0x0 0xfdc70000 0x0 0x800>;
> --
> 2.44.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] media: dt-binding: media: Document rk3588’s VEPU121
From: Sebastian Reichel @ 2024-03-27 17:44 UTC (permalink / raw)
To: Emmanuel Gil Peyrot
Cc: linux-kernel, Ezequiel Garcia, Philipp Zabel,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
Robin Murphy, Cristian Ciocaltea, Dragan Simic, Shreeya Patel,
Chris Morgan, Andy Yan, Nicolas Frattaroli, linux-media,
linux-rockchip, devicetree, linux-arm-kernel, iommu
In-Reply-To: <20240327134115.424846-2-linkmauve@linkmauve.fr>
[-- Attachment #1: Type: text/plain, Size: 1518 bytes --]
Hi,
On Wed, Mar 27, 2024 at 02:41:11PM +0100, Emmanuel Gil Peyrot wrote:
> This encoder-only device is present four times on this SoC, and should
> support everything the rk3568 vepu supports (so JPEG, H.264 and VP8
> encoding).
>
> According to the TRM[1], there is also the VEPU580 encoder which
> supports H.264 and H.265, and various VDPU* decoders, of which only the
> VDPU981 is currently supported. This patch describes only the VEPU121.
>
> [1] https://github.com/FanX-Tek/rk3588-TRM-and-Datasheet
>
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> .../devicetree/bindings/media/rockchip,rk3568-vepu.yaml | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> index 9d90d8d0565a..4c6cb21da041 100644
> --- a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> @@ -15,8 +15,12 @@ description:
>
> properties:
> compatible:
> - enum:
> - - rockchip,rk3568-vepu
> + oneOf:
> + - const: rockchip,rk3568-vepu
> + - items:
> + - enum:
> + - rockchip,rk3588-vepu121
> + - const: rockchip,rk3568-vepu
>
> reg:
> maxItems: 1
> --
> 2.44.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: display: bridge: it6505: Add #sound-dai-cells
From: Rob Herring @ 2024-03-27 17:33 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Laurent Pinchart, Krzysztof Kozlowski, dri-devel, Maxime Ripard,
Neil Armstrong, Daniel Vetter, Robert Foss, Rob Herring,
Conor Dooley, Thomas Zimmermann, David Airlie, linux-kernel,
devicetree, Jonas Karlman, Andrzej Hajda, Jernej Skrabec,
Maarten Lankhorst
In-Reply-To: <20240327085250.3427496-1-wenst@chromium.org>
On Wed, 27 Mar 2024 16:52:48 +0800, Chen-Yu Tsai wrote:
> The ITE IT6505 display bridge can take one I2S input and transmit it
> over the DisplayPort link.
>
> Add #sound-dai-cells (= 0) to the binding for it.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> Changes since v1 [1]:
> - Reference /schemas/sound/dai-common.yaml
> - Change "additionalProperties: false" to "unevaluatedProperties: false"
>
> The driver side changes [2] are still being worked on.
>
> [1] https://lore.kernel.org/dri-devel/20240126073511.2708574-1-wenst@chromium.org/
> [2] https://lore.kernel.org/linux-arm-kernel/20230730180803.22570-4-jiaxin.yu@mediatek.com/
> ---
> .../devicetree/bindings/display/bridge/ite,it6505.yaml | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 1/2] ASoC: dt-bindings: Added schema for "nuvoton,nau8325"
From: Rob Herring @ 2024-03-27 17:32 UTC (permalink / raw)
To: Seven Lee
Cc: supercraig0719, conor+dt, YHCHuang, linux-kernel,
krzysztof.kozlowski+dt, SJLIN0, scott6986, perex, tiwai, robh+dt,
broonie, alsa-devel, lgirdwood, dardar923, devicetree, CTLIN0,
linux-sound, KCHSU0
In-Reply-To: <20240327075755.3410381-2-wtli@nuvoton.com>
On Wed, 27 Mar 2024 15:57:54 +0800, Seven Lee wrote:
> Add a DT schema for describing nau8325 audio amplifiers. The key features
> are as follows:
> - Low SPK_VDD Quiescent Current
> - Gain Setting with 2-wire interface
> - Powerful Stereo Class-D Amplifier
> - Low Output Noise
> - Low Current Shutdown Mode
> - Click-and Pop Suppression
>
> More resources :
> https://www.nuvoton.com/products/smart-home-audio/audio-amplifiers/class-d-series/nau8325yg/
>
> Signed-off-by: Seven Lee <wtli@nuvoton.com>
> ---
> .../bindings/sound/nuvoton,nau8325.yaml | 80 +++++++++++++++++++
> 1 file changed, 80 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/nuvoton,nau8325.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RFC PATCH 02/13] dt-bindings: pinctrl: renesas: Document RZ/V2H(P) SoC
From: Rob Herring @ 2024-03-27 17:24 UTC (permalink / raw)
To: Prabhakar
Cc: Geert Uytterhoeven, Linus Walleij, Krzysztof Kozlowski,
Conor Dooley, Magnus Damm, linux-renesas-soc, linux-gpio,
devicetree, linux-kernel, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20240326222844.1422948-3-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Tue, Mar 26, 2024 at 10:28:33PM +0000, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add documentation for the pin controller found on the Renesas RZ/V2H(P)
> (R9A09G057) SoC. Compared to RZ/G2L family of SoCs there are slight
> differences on the RZ/V2H(P) SoC for pinmuxing.
>
> Also add 'renesas-rzv2h,output-impedance' property. Drive strength
> setting on RZ/V2H(P) depends on the different power rails which are
> coming out from the PMIC (connected via i2c). These power rails
> (required for drive strength) can be 1.2/1.8/3.3V.
>
> Pin are grouped into 4 groups,
>
> Group1: Impedance
> - 150/75/38/25 ohms (at 3.3 V)
> - 130/65/33/22 ohms (at 1.8 V)
>
> Group2: Impedance
> - 50/40/33/25 ohms (at 1.8 V)
>
> Group3: Impedance
> - 150/75/37.5/25 ohms (at 3.3 V)
> - 130/65/33/22 ohms (at 1.8 V)
>
> Group4: Impedance
> - 110/55/30/20 ohms (at 1.8 V)
> - 150/75/38/25 ohms (at 1.2 V)
>
> 'renesas-rzv2h,output-impedance' property as documented which can be
> [1, 2, 4, 6] indicates x Value strength.
Looks like the values are x1, x1.5, x3ish, x6...
>
> As the power rail information cannot be available very early in the
> boot process as 'renesas-rzv2h,output-impedance' property is added
> instead of reusing output-impedance-ohms property.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> .../pinctrl/renesas,rzg2l-pinctrl.yaml | 22 +++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> index 881e992adca3..77f4fc7f4a21 100644
> --- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> @@ -26,6 +26,7 @@ properties:
> - renesas,r9a07g043-pinctrl # RZ/G2UL{Type-1,Type-2} and RZ/Five
> - renesas,r9a07g044-pinctrl # RZ/G2{L,LC}
> - renesas,r9a08g045-pinctrl # RZ/G3S
> + - renesas,r9a09g057-pinctrl # RZ/V2H(P)
>
> - items:
> - enum:
> @@ -66,10 +67,14 @@ properties:
> maxItems: 1
>
> resets:
> - items:
> - - description: GPIO_RSTN signal
> - - description: GPIO_PORT_RESETN signal
> - - description: GPIO_SPARE_RESETN signal
> + oneOf:
> + - items:
> + - description: GPIO_RSTN signal
> + - description: GPIO_PORT_RESETN signal
> + - description: GPIO_SPARE_RESETN signal
> + - items:
> + - description: PFC main reset
> + - description: Reset for the control register related to WDTUDFCA and WDTUDFFCM pins
>
> additionalProperties:
> anyOf:
> @@ -111,6 +116,15 @@ additionalProperties:
> output-high: true
> output-low: true
> line-name: true
> + renesas-rzv2h,output-impedance:
'renesas-rzv2h' is not a vendor.
That should give you a warning if you actually used this somewhere.
> + description: |
> + Output impedance for pins on RZ/V2H(P) SoC.
> + x1: Corresponds to 0 in IOLH register.
> + x2: Corresponds to 1 in IOLH register.
> + x4: Corresponds to 2 in IOLH register.
> + x6: Corresponds to 3 in IOLH register.
Why not just use 0-3 for the values?
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2, 4, 6]
>
> - type: object
> additionalProperties:
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v2 1/2] media: dt-binding: media: Document rk3588’s VEPU121
From: Conor Dooley @ 2024-03-27 17:23 UTC (permalink / raw)
To: Emmanuel Gil Peyrot
Cc: linux-kernel, Ezequiel Garcia, Philipp Zabel,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
Robin Murphy, Sebastian Reichel, Cristian Ciocaltea, Dragan Simic,
Shreeya Patel, Chris Morgan, Andy Yan, Nicolas Frattaroli,
linux-media, linux-rockchip, devicetree, linux-arm-kernel, iommu
In-Reply-To: <20240327134115.424846-2-linkmauve@linkmauve.fr>
[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]
On Wed, Mar 27, 2024 at 02:41:11PM +0100, Emmanuel Gil Peyrot wrote:
> This encoder-only device is present four times on this SoC, and should
> support everything the rk3568 vepu supports (so JPEG, H.264 and VP8
> encoding).
>
> According to the TRM[1], there is also the VEPU580 encoder which
> supports H.264 and H.265, and various VDPU* decoders, of which only the
> VDPU981 is currently supported. This patch describes only the VEPU121.
>
> [1] https://github.com/FanX-Tek/rk3588-TRM-and-Datasheet
>
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.
> ---
> .../devicetree/bindings/media/rockchip,rk3568-vepu.yaml | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> index 9d90d8d0565a..4c6cb21da041 100644
> --- a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> @@ -15,8 +15,12 @@ description:
>
> properties:
> compatible:
> - enum:
> - - rockchip,rk3568-vepu
> + oneOf:
> + - const: rockchip,rk3568-vepu
> + - items:
> + - enum:
> + - rockchip,rk3588-vepu121
> + - const: rockchip,rk3568-vepu
>
> reg:
> maxItems: 1
> --
> 2.44.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] phy: rockchip: emmc: Enable pulldown for strobe line
From: Conor Dooley @ 2024-03-27 17:21 UTC (permalink / raw)
To: Folker Schwesinger
Cc: Dragan Simic, Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner,
Chris Ruehl, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Christopher Obbard, Alban Browaeys, Doug Anderson, Brian Norris,
Jensen Huang, linux-phy, linux-arm-kernel, linux-rockchip,
linux-kernel, devicetree
In-Reply-To: <D04O57YQHYI4.1QNGSVKVT44CS@folker-schwesinger.de>
[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]
On Wed, Mar 27, 2024 at 04:21:45PM +0000, Folker Schwesinger wrote:
> Hi Conor and Dragan,
>
> thanks for your feedback!
>
> On Tue Mar 26, 2024 at 8:55 PM CET, Dragan Simic wrote:
> > On 2024-03-26 20:46, Conor Dooley wrote:
> > > On Tue, Mar 26, 2024 at 07:54:35PM +0100, Folker Schwesinger via B4
> > > Relay wrote:
> > >> From: Folker Schwesinger <dev@folker-schwesinger.de>
> > >>
> > >> Restore the behavior of the Rockchip kernel that undconditionally
> > >> enables the internal strobe pulldown.
> > >
> > > What do you mean "restore the behaviour of the rockchip kernel"? Did
> > > mainline behave the same as the rockchip kernel previously? If not,
> > > using "restore" here is misleading. "Unconditionally" is also
> > > incorrect,
> > > because you have a property that disables it.
>
> Apologizes for the misleading commit message. Prior to 5.11 the Linux
> kernel did not touch the pull-down registers. However, it seems the
> register's (factory?) default was set to enable the pull-down. As it
> was mentioned elsewhere that was the configuration recommended by
> Rockchip. The 4.4 vendor (Rockchip) kernel reflects that by enabling the
> pull-down in its kernel.
Yeah, seems like a bit of a sticky situation. Probably the wrong
polarity was chosen when the property was implemented and the property
should have been the one you wanted to switch to given the default
before it existed was the factory defaults.
> Of course, this has nothing to do with the Linux kernel, so "restore"
> was a bad choice here.
>
> I previously had split the driver patch into two separate patches, one
> for changing the default (unconditionally at that point), the other for
> adding the disable property. As both changes were minimal I decided to
> squash the commits. I updated the cover letter, but forgot to update the
> commit message. Sorry.
No worries. Squashing them was probably the right thing to do anyway.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 5/6] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB Type-C display
From: Konrad Dybcio @ 2024-03-27 17:20 UTC (permalink / raw)
To: Bjorn Andersson, cros-qcom-dts-watchers, Bjorn Andersson,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Will Deacon, Dmitry Baryshkov
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel,
Neil Armstrong, Krishna Kurapati PSSNV
In-Reply-To: <20240326-rb3gen2-dp-connector-v2-5-a9f1bc32ecaf@quicinc.com>
On 27.03.2024 3:04 AM, Bjorn Andersson wrote:
> With the ADSP remoteproc loaded pmic_glink can be introduced and
> together with the redriver wired up to provide role and orientation
> switching signals as well as USB Type-C display on the RB3gen2.
>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Tested-By: Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v2 4/6] arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver
From: Konrad Dybcio @ 2024-03-27 17:20 UTC (permalink / raw)
To: Bjorn Andersson, cros-qcom-dts-watchers, Bjorn Andersson,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Will Deacon, Dmitry Baryshkov
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20240326-rb3gen2-dp-connector-v2-4-a9f1bc32ecaf@quicinc.com>
On 27.03.2024 3:04 AM, Bjorn Andersson wrote:
> The RB3gen2 has a USB redriver on APPS_I2C, enable the bus and introduce
> the redriver. The plumbing with other components is kept separate for
> clarity.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v2 2/6] arm64: dts: qcom: qcs6490-rb3gen2: Add DP output
From: Konrad Dybcio @ 2024-03-27 17:20 UTC (permalink / raw)
To: Bjorn Andersson, cros-qcom-dts-watchers, Bjorn Andersson,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Will Deacon, Dmitry Baryshkov
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20240326-rb3gen2-dp-connector-v2-2-a9f1bc32ecaf@quicinc.com>
On 27.03.2024 3:04 AM, Bjorn Andersson wrote:
> The RB3Gen2 board comes with a mini DP connector, describe this, enable
> MDSS, DP controller and the PHY that drives this.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [RFC PATCH 01/13] dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Remove the check from the object
From: Rob Herring @ 2024-03-27 17:16 UTC (permalink / raw)
To: Prabhakar
Cc: Conor Dooley, Linus Walleij, devicetree, linux-gpio,
Krzysztof Kozlowski, Geert Uytterhoeven, linux-renesas-soc,
linux-kernel, Fabrizio Castro, Magnus Damm, Lad Prabhakar
In-Reply-To: <20240326222844.1422948-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Tue, 26 Mar 2024 22:28:32 +0000, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Drop the bogus check from object as this didn't really add restriction check.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> .../bindings/pinctrl/renesas,rzg2l-pinctrl.yaml | 15 ---------------
> 1 file changed, 15 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 4/4] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
From: Mathieu Poirier @ 2024-03-27 17:14 UTC (permalink / raw)
To: Arnaud POULIQUEN
Cc: Bjorn Andersson, Jens Wiklander, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-stm32, linux-arm-kernel, linux-remoteproc,
linux-kernel, op-tee, devicetree
In-Reply-To: <a08add21-b8ff-434a-9689-6af8b05b1965@foss.st.com>
On Tue, Mar 26, 2024 at 08:31:33PM +0100, Arnaud POULIQUEN wrote:
>
>
> On 3/25/24 17:51, Mathieu Poirier wrote:
> > On Fri, Mar 08, 2024 at 03:47:08PM +0100, Arnaud Pouliquen wrote:
> >> The new TEE remoteproc device is used to manage remote firmware in a
> >> secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is
> >> introduced to delegate the loading of the firmware to the trusted
> >> execution context. In such cases, the firmware should be signed and
> >> adhere to the image format defined by the TEE.
> >>
> >> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
> >> ---
> >> Updates from V3:
> >> - remove support of the attach use case. Will be addressed in a separate
> >> thread,
> >> - add st_rproc_tee_ops::parse_fw ops,
> >> - inverse call of devm_rproc_alloc()and tee_rproc_register() to manage cross
> >> reference between the rproc struct and the tee_rproc struct in tee_rproc.c.
> >> ---
> >> drivers/remoteproc/stm32_rproc.c | 60 +++++++++++++++++++++++++++++---
> >> 1 file changed, 56 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> >> index 8cd838df4e92..13df33c78aa2 100644
> >> --- a/drivers/remoteproc/stm32_rproc.c
> >> +++ b/drivers/remoteproc/stm32_rproc.c
> >> @@ -20,6 +20,7 @@
> >> #include <linux/remoteproc.h>
> >> #include <linux/reset.h>
> >> #include <linux/slab.h>
> >> +#include <linux/tee_remoteproc.h>
> >> #include <linux/workqueue.h>
> >>
> >> #include "remoteproc_internal.h"
> >> @@ -49,6 +50,9 @@
> >> #define M4_STATE_STANDBY 4
> >> #define M4_STATE_CRASH 5
> >>
> >> +/* Remote processor unique identifier aligned with the Trusted Execution Environment definitions */
> >
> > Why is this the case? At least from the kernel side it is possible to call
> > tee_rproc_register() with any kind of value, why is there a need to be any
> > kind of alignment with the TEE?
>
>
> The use of the proc_id is to identify a processor in case of multi co-processors.
>
That is well understood.
> For instance we can have a system with A DSP and a modem. We would use the same
> TEE service, but
That too.
> the TEE driver will probably be different, same for the signature key.
What TEE driver are we talking about here?
> In such case the proc ID allows to identify the the processor you want to address.
>
That too is well understood, but there is no alignment needed with the TEE, i.e
the TEE application is not expecting a value of '0'. We could set
STM32_MP1_M4_PROC_ID to 0xDEADBEEF and things would work. This driver won't go
anywhere for as long as it is not the case.
>
> >
> >> +#define STM32_MP1_M4_PROC_ID 0
> >> +
> >> struct stm32_syscon {
> >> struct regmap *map;
> >> u32 reg;
> >> @@ -257,6 +261,19 @@ static int stm32_rproc_release(struct rproc *rproc)
> >> return 0;
> >> }
> >>
> >> +static int stm32_rproc_tee_stop(struct rproc *rproc)
> >> +{
> >> + int err;
> >> +
> >> + stm32_rproc_request_shutdown(rproc);
> >> +
> >> + err = tee_rproc_stop(rproc);
> >> + if (err)
> >> + return err;
> >> +
> >> + return stm32_rproc_release(rproc);
> >> +}
> >> +
> >> static int stm32_rproc_prepare(struct rproc *rproc)
> >> {
> >> struct device *dev = rproc->dev.parent;
> >> @@ -693,8 +710,19 @@ static const struct rproc_ops st_rproc_ops = {
> >> .get_boot_addr = rproc_elf_get_boot_addr,
> >> };
> >>
> >> +static const struct rproc_ops st_rproc_tee_ops = {
> >> + .prepare = stm32_rproc_prepare,
> >> + .start = tee_rproc_start,
> >> + .stop = stm32_rproc_tee_stop,
> >> + .kick = stm32_rproc_kick,
> >> + .load = tee_rproc_load_fw,
> >> + .parse_fw = tee_rproc_parse_fw,
> >> + .find_loaded_rsc_table = tee_rproc_find_loaded_rsc_table,
> >> +};
> >> +
> >> static const struct of_device_id stm32_rproc_match[] = {
> >> - { .compatible = "st,stm32mp1-m4" },
> >> + {.compatible = "st,stm32mp1-m4",},
> >> + {.compatible = "st,stm32mp1-m4-tee",},
> >> {},
> >> };
> >> MODULE_DEVICE_TABLE(of, stm32_rproc_match);
> >> @@ -853,6 +881,7 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> >> struct device *dev = &pdev->dev;
> >> struct stm32_rproc *ddata;
> >> struct device_node *np = dev->of_node;
> >> + struct tee_rproc *trproc = NULL;
> >> struct rproc *rproc;
> >> unsigned int state;
> >> int ret;
> >> @@ -861,9 +890,26 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> >> if (ret)
> >> return ret;
> >>
> >> - rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
> >> - if (!rproc)
> >> - return -ENOMEM;
> >> + if (of_device_is_compatible(np, "st,stm32mp1-m4-tee")) {
> >> + /*
> >> + * Delegate the firmware management to the secure context.
> >> + * The firmware loaded has to be signed.
> >> + */
> >> + rproc = devm_rproc_alloc(dev, np->name, &st_rproc_tee_ops, NULL, sizeof(*ddata));
> >> + if (!rproc)
> >> + return -ENOMEM;
> >> +
> >> + trproc = tee_rproc_register(dev, rproc, STM32_MP1_M4_PROC_ID);
> >> + if (IS_ERR(trproc)) {
> >> + dev_err_probe(dev, PTR_ERR(trproc),
> >> + "signed firmware not supported by TEE\n");
> >> + return PTR_ERR(trproc);
> >> + }
> >> + } else {
> >> + rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
> >> + if (!rproc)
> >> + return -ENOMEM;
> >> + }
> >>
> >> ddata = rproc->priv;
> >>
> >> @@ -915,6 +961,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> >> dev_pm_clear_wake_irq(dev);
> >> device_init_wakeup(dev, false);
> >> }
> >> + if (trproc)
> >
> > if (rproc->tee_interface)
> >
> >
> > I am done reviewing this set.
>
> Thank for your review!
> Arnaud
>
> >
> > Thanks,
> > Mathieu
> >
> >> + tee_rproc_unregister(trproc);
> >> +
> >> return ret;
> >> }
> >>
> >> @@ -935,6 +984,9 @@ static void stm32_rproc_remove(struct platform_device *pdev)
> >> dev_pm_clear_wake_irq(dev);
> >> device_init_wakeup(dev, false);
> >> }
> >> + if (rproc->tee_interface)
> >> + tee_rproc_unregister(rproc->tee_interface);
> >> +
> >> }
> >>
> >> static int stm32_rproc_suspend(struct device *dev)
> >> --
> >> 2.25.1
> >>
^ permalink raw reply
* Re: [PATCH v2 4/4] pinctrl: qcom: spmi-gpio: Add PMIH0108 and PMD8028 support
From: Konrad Dybcio @ 2024-03-27 17:13 UTC (permalink / raw)
To: Anjelique Melendez, andersson, linus.walleij, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel,
quic_subbaram, quic_collinsd, quic_jprakash
In-Reply-To: <20240326220628.2392802-5-quic_amelende@quicinc.com>
On 26.03.2024 11:06 PM, Anjelique Melendez wrote:
> Add support for qcom,pmih0108-gpio and qcom,pmd8028-gpio.
>
> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v2 3/4] pinctrl: qcom: spmi-gpio: Add PMXR2230 and PM6450 support
From: Konrad Dybcio @ 2024-03-27 17:13 UTC (permalink / raw)
To: Anjelique Melendez, andersson, linus.walleij, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel,
quic_subbaram, quic_collinsd, quic_jprakash
In-Reply-To: <20240326220628.2392802-4-quic_amelende@quicinc.com>
On 26.03.2024 11:06 PM, Anjelique Melendez wrote:
> Add support for qcom,pmxr2230-gpio and qcom,pm6450-gpio.
>
> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v4 1/4] remoteproc: Add TEE support
From: Mathieu Poirier @ 2024-03-27 17:07 UTC (permalink / raw)
To: Arnaud POULIQUEN
Cc: Bjorn Andersson, Jens Wiklander, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-stm32, linux-arm-kernel, linux-remoteproc,
linux-kernel, op-tee, devicetree
In-Reply-To: <5bb9f583-351d-45a4-90e1-ce0b8dde8ce2@foss.st.com>
On Tue, Mar 26, 2024 at 08:18:23PM +0100, Arnaud POULIQUEN wrote:
> Hello Mathieu,
>
> On 3/25/24 17:46, Mathieu Poirier wrote:
> > On Fri, Mar 08, 2024 at 03:47:05PM +0100, Arnaud Pouliquen wrote:
> >> Add a remoteproc TEE (Trusted Execution Environment) driver
> >> that will be probed by the TEE bus. If the associated Trusted
> >> application is supported on secure part this device offers a client
> >
> > Device or driver? I thought I touched on that before.
>
> Right, I changed the first instance and missed this one
>
> >
> >> interface to load a firmware in the secure part.
> >> This firmware could be authenticated by the secure trusted application.
> >>
> >> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
> >> ---
> >> Updates from V3:
> >> - rework TEE_REMOTEPROC description in Kconfig
> >> - fix some namings
> >> - add tee_rproc_parse_fw to support rproc_ops::parse_fw
> >> - add proc::tee_interface;
> >> - add rproc struct as parameter of the tee_rproc_register() function
> >> ---
> >> drivers/remoteproc/Kconfig | 10 +
> >> drivers/remoteproc/Makefile | 1 +
> >> drivers/remoteproc/tee_remoteproc.c | 434 ++++++++++++++++++++++++++++
> >> include/linux/remoteproc.h | 4 +
> >> include/linux/tee_remoteproc.h | 112 +++++++
> >> 5 files changed, 561 insertions(+)
> >> create mode 100644 drivers/remoteproc/tee_remoteproc.c
> >> create mode 100644 include/linux/tee_remoteproc.h
> >>
> >> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> >> index 48845dc8fa85..2cf1431b2b59 100644
> >> --- a/drivers/remoteproc/Kconfig
> >> +++ b/drivers/remoteproc/Kconfig
> >> @@ -365,6 +365,16 @@ config XLNX_R5_REMOTEPROC
> >>
> >> It's safe to say N if not interested in using RPU r5f cores.
> >>
> >> +
> >> +config TEE_REMOTEPROC
> >> + tristate "remoteproc support by a TEE application"
> >
> > s/remoteproc/Remoteproc
> >
> >> + depends on OPTEE
> >> + help
> >> + Support a remote processor with a TEE application. The Trusted
> >> + Execution Context is responsible for loading the trusted firmware
> >> + image and managing the remote processor's lifecycle.
> >> + This can be either built-in or a loadable module.
> >> +
> >> endif # REMOTEPROC
> >>
> >> endmenu
> >> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> >> index 91314a9b43ce..fa8daebce277 100644
> >> --- a/drivers/remoteproc/Makefile
> >> +++ b/drivers/remoteproc/Makefile
> >> @@ -36,6 +36,7 @@ obj-$(CONFIG_RCAR_REMOTEPROC) += rcar_rproc.o
> >> obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
> >> obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
> >> obj-$(CONFIG_STM32_RPROC) += stm32_rproc.o
> >> +obj-$(CONFIG_TEE_REMOTEPROC) += tee_remoteproc.o
> >> obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o
> >> obj-$(CONFIG_TI_K3_R5_REMOTEPROC) += ti_k3_r5_remoteproc.o
> >> obj-$(CONFIG_XLNX_R5_REMOTEPROC) += xlnx_r5_remoteproc.o
> >> diff --git a/drivers/remoteproc/tee_remoteproc.c b/drivers/remoteproc/tee_remoteproc.c
> >> new file mode 100644
> >> index 000000000000..c855210e52e3
> >> --- /dev/null
> >> +++ b/drivers/remoteproc/tee_remoteproc.c
> >> @@ -0,0 +1,434 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later
> >> +/*
> >> + * Copyright (C) STMicroelectronics 2024 - All Rights Reserved
> >> + * Author: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
> >> + */
> >> +
> >> +#include <linux/firmware.h>
> >> +#include <linux/io.h>
> >> +#include <linux/module.h>
> >> +#include <linux/remoteproc.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/tee_drv.h>
> >> +#include <linux/tee_remoteproc.h>
> >> +
> >> +#include "remoteproc_internal.h"
> >> +
> >> +#define MAX_TEE_PARAM_ARRY_MEMBER 4
> >> +
> >> +/*
> >> + * Authentication of the firmware and load in the remote processor memory
> >> + *
> >> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> >> + * [in] params[1].memref: buffer containing the image of the buffer
> >> + */
> >> +#define TA_RPROC_FW_CMD_LOAD_FW 1
> >> +
> >> +/*
> >> + * Start the remote processor
> >> + *
> >> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> >> + */
> >> +#define TA_RPROC_FW_CMD_START_FW 2
> >> +
> >> +/*
> >> + * Stop the remote processor
> >> + *
> >> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> >> + */
> >> +#define TA_RPROC_FW_CMD_STOP_FW 3
> >> +
> >> +/*
> >> + * Return the address of the resource table, or 0 if not found
> >> + * No check is done to verify that the address returned is accessible by
> >> + * the non secure context. If the resource table is loaded in a protected
> >> + * memory the access by the non secure context will lead to a data abort.
> >> + *
> >> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> >> + * [out] params[1].value.a: 32bit LSB resource table memory address
> >> + * [out] params[1].value.b: 32bit MSB resource table memory address
> >> + * [out] params[2].value.a: 32bit LSB resource table memory size
> >> + * [out] params[2].value.b: 32bit MSB resource table memory size
> >> + */
> >> +#define TA_RPROC_FW_CMD_GET_RSC_TABLE 4
> >> +
> >> +/*
> >> + * Return the address of the core dump
> >> + *
> >> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> >> + * [out] params[1].memref: address of the core dump image if exist,
> >> + * else return Null
> >> + */
> >> +#define TA_RPROC_FW_CMD_GET_COREDUMP 5
> >> +
> >> +struct tee_rproc_context {
> >> + struct list_head sessions;
> >> + struct tee_context *tee_ctx;
> >> + struct device *dev;
> >> +};
> >> +
> >> +static struct tee_rproc_context *tee_rproc_ctx;
> >> +
> >> +static void tee_rproc_prepare_args(struct tee_rproc *trproc, int cmd,
> >> + struct tee_ioctl_invoke_arg *arg,
> >> + struct tee_param *param,
> >> + unsigned int num_params)
> >> +{
> >> + memset(arg, 0, sizeof(*arg));
> >> + memset(param, 0, MAX_TEE_PARAM_ARRY_MEMBER * sizeof(*param));
> >> +
> >> + arg->func = cmd;
> >> + arg->session = trproc->session_id;
> >> + arg->num_params = num_params + 1;
> >> +
> >> + param[0] = (struct tee_param) {
> >> + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> >> + .u.value.a = trproc->rproc_id,
> >> + };
> >> +}
> >> +
> >> +int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw)
> >> +{
> >> + struct tee_ioctl_invoke_arg arg;
> >> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> >> + struct tee_rproc *trproc = rproc->tee_interface;
> >> + struct tee_shm *fw_shm;
> >> + int ret;
> >
> > Declarations in reverse ascending order here and everywhere in the driver.
> > Sometimes it is done properly, sometimes it isn't.
> >
> >> +
> >> + if (!trproc)
> >> + return -EINVAL;
> >> +
> >> + fw_shm = tee_shm_register_kernel_buf(tee_rproc_ctx->tee_ctx, (void *)fw->data, fw->size);
> >> + if (IS_ERR(fw_shm))
> >> + return PTR_ERR(fw_shm);
> >> +
> >> + tee_rproc_prepare_args(trproc, TA_RPROC_FW_CMD_LOAD_FW, &arg, param, 1);
> >> +
> >> + /* Provide the address of the firmware image */
> >> + param[1] = (struct tee_param) {
> >> + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT,
> >> + .u.memref = {
> >> + .shm = fw_shm,
> >> + .size = fw->size,
> >> + .shm_offs = 0,
> >> + },
> >> + };
> >> +
> >> + ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
> >> + if (ret < 0 || arg.ret != 0) {
> >> + dev_err(tee_rproc_ctx->dev,
> >> + "TA_RPROC_FW_CMD_LOAD_FW invoke failed TEE err: %x, ret:%x\n",
> >> + arg.ret, ret);
> >> + if (!ret)
> >> + ret = -EIO;
> >> + }
> >> +
> >> + tee_shm_free(fw_shm);
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tee_rproc_load_fw);
> >> +
> >> +struct resource_table *tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
> >> +{
> >> + struct tee_ioctl_invoke_arg arg;
> >> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> >> + struct tee_rproc *trproc = rproc->tee_interface;
> >> + struct resource_table *rsc_table;
> >> + int ret;
> >> +
> >> + if (!trproc)
> >> + return ERR_PTR(-EINVAL);
> >> +
> >> + tee_rproc_prepare_args(trproc, TA_RPROC_FW_CMD_GET_RSC_TABLE, &arg, param, 2);
> >> +
> >> + param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> >> + param[2].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> >> +
> >> + ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
> >> + if (ret < 0 || arg.ret != 0) {
> >> + dev_err(tee_rproc_ctx->dev,
> >> + "TA_RPROC_FW_CMD_GET_RSC_TABLE invoke failed TEE err: %x, ret:%x\n",
> >> + arg.ret, ret);
> >> + return ERR_PTR(-EIO);
> >> + }
> >> +
> >> + *table_sz = param[2].u.value.a;
> >> +
> >> + /* If the size is null no resource table defined in the image */
> >> + if (!*table_sz)
> >> + return NULL;
> >> +
> >> + /* Store the resource table address that would be updated by the remote core. */
> >> + rsc_table = ioremap_wc(param[1].u.value.a, *table_sz);
> >> + if (IS_ERR_OR_NULL(rsc_table)) {
> >> + dev_err(tee_rproc_ctx->dev, "Unable to map memory region: %lld+%zx\n",
> >> + param[1].u.value.a, *table_sz);
> >> + return ERR_PTR(-ENOMEM);
> >> + }
> >> +
> >> + return rsc_table;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tee_rproc_get_loaded_rsc_table);
> >> +
> >> +int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> >> +{
> >> + struct tee_rproc *trproc = rproc->tee_interface;
> >> + struct resource_table *rsc_table;
> >> + size_t table_sz;
> >> + int ret;
> >> +
> >> + ret = tee_rproc_load_fw(rproc, fw);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + rsc_table = tee_rproc_get_loaded_rsc_table(rproc, &table_sz);
> >> + if (IS_ERR(rsc_table))
> >> + return PTR_ERR(rsc_table);
> >> +
> >> + /* Create a copy of the resource table to have same behaviour than the elf loader. */
> >> + rproc->cached_table = kmemdup(rsc_table, table_sz, GFP_KERNEL);
> >> + if (!rproc->cached_table)
> >> + return -ENOMEM;
> >
> > Why not ->table_ptr and setting ->cached_table to NULL?
>
> It was my plan preparing this version. But during implementarion it looks
> to me that having exactly same behavior than the ELF loader would be
> simpler to maintain the remoteproc avoiding to update in the remoteproc core
> to manage for the cached resource table (see also my comment below abourt recovery)
> That why I propose this implementation
>
> That said what you proposal should also work (with some updates in
> remoteproc_core for the management of the cached table).
>
Yes
> So please just comfirm your preference.
>
Definitely keep ->cached_table to NULL.
> >
> >> +
> >> + rproc->table_ptr = rproc->cached_table;
> >> + rproc->table_sz = table_sz;
> >> + trproc->rsc_table = rsc_table;
> >
> > I really don't see why this is needed, please remove and use rproc->table_ptr
> > instead.
>
> I need to store it for the iounmap in tee_rproc_remove.
iounmap(entry->rproc->rsc_table);
What am I missing?
>
> >
> >> +
> >> + return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tee_rproc_parse_fw);
> >> +
> >> +struct resource_table *tee_rproc_find_loaded_rsc_table(struct rproc *rproc,
> >> + const struct firmware *fw)
> >> +{
> >> + struct tee_rproc *trproc = rproc->tee_interface;
> >> + struct resource_table *rsc_table;
> >> + size_t table_sz;
> >> +
> >> + if (!trproc)
> >> + return ERR_PTR(-EINVAL);
> >> +
> >> + /* Check if the resourse table has already been obtained in tee_rproc_parse_fw() */
> >> + if (trproc->rsc_table)
> >> + return trproc->rsc_table;
> >
> > Again, why not simply use rproc->rsc_table? This function should only return
> > the resource table that was set in tee_rproc_parse_fw().
>
> In case of recovery rproc->_rsc_table point to the cached table [1]
In [1], on line 1554, add a check for rproc->tee_interface and if it is valid
call rproc_find_loaded_rsc_table().
> and we need to reapply the configuration in rproc_start() called during the
> recovery[2]
1) Rename rproc_set_rsc_table() to rproc_set_rsc_table_on_attach()
2) Introduce a new function called rproc_set_rsc_table_on_start()
3) Move code from [2], line 1278 to 1292, to that new function. In the new
function, add a check on rproc->tee_interface. If it is valid then call
rproc_find_loaded_rsc_table().
4) in rproc_start(), replace lines 1278 to 1292 with a call to
rproc_set_rsc_table_on_start().
> [1]https://elixir.bootlin.com/linux/latest/source/drivers/remoteproc/remoteproc_core.c#L1586
> [2]https://elixir.bootlin.com/linux/latest/source/drivers/remoteproc/remoteproc_core.c#L1287
>
> >
> >> +
> >> + rsc_table = tee_rproc_get_loaded_rsc_table(rproc, &table_sz);
> >> + if (IS_ERR(rsc_table))
> >> + return rsc_table;
> >> +
> >> + rproc->table_sz = table_sz;
> >> + trproc->rsc_table = rsc_table;
> >> +
> >> + return rsc_table;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tee_rproc_find_loaded_rsc_table);
> >> +
> >> +int tee_rproc_start(struct rproc *rproc)
> >> +{
> >> + struct tee_ioctl_invoke_arg arg;
> >> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> >> + struct tee_rproc *trproc = rproc->tee_interface;
> >> + int ret;
> >> +
> >> + if (!trproc)
> >> + return -EINVAL;
> >> +
> >> + tee_rproc_prepare_args(trproc, TA_RPROC_FW_CMD_START_FW, &arg, param, 0);
> >> +
> >> + ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
> >> + if (ret < 0 || arg.ret != 0) {
> >> + dev_err(tee_rproc_ctx->dev,
> >> + "TA_RPROC_FW_CMD_START_FW invoke failed TEE err: %x, ret:%x\n",
> >> + arg.ret, ret);
> >> + if (!ret)
> >> + ret = -EIO;
> >> + }
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tee_rproc_start);
> >> +
> >> +int tee_rproc_stop(struct rproc *rproc)
> >> +{
> >> + struct tee_ioctl_invoke_arg arg;
> >> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> >> + struct tee_rproc *trproc = rproc->tee_interface;
> >> + int ret;
> >> +
> >> + if (!trproc)
> >> + return -EINVAL;
> >> +
> >> + tee_rproc_prepare_args(trproc, TA_RPROC_FW_CMD_STOP_FW, &arg, param, 0);
> >> +
> >> + ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
> >> + if (ret < 0 || arg.ret != 0) {
> >> + dev_err(tee_rproc_ctx->dev,
> >> + "TA_RPROC_FW_CMD_STOP_FW invoke failed TEE err: %x, ret:%x\n",
> >> + arg.ret, ret);
> >> + if (!ret)
> >> + ret = -EIO;
> >> + }
> >> +
> >> + if (!rproc->table_ptr)
> >> + return ret;
> >> +
> >> + iounmap(trproc->rsc_table);
> >> + trproc->rsc_table = NULL;
> >> + rproc->table_ptr = NULL;
> >> +
> >> + return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tee_rproc_stop);
> >> +
> >> +static const struct tee_client_device_id stm32_tee_rproc_id_table[] = {
> >> + {UUID_INIT(0x80a4c275, 0x0a47, 0x4905,
> >> + 0x82, 0x85, 0x14, 0x86, 0xa9, 0x77, 0x1a, 0x08)},
> >> + {}
> >> +};
> >> +
> >> +struct tee_rproc *tee_rproc_register(struct device *dev, struct rproc *rproc, unsigned int rproc_id)
> >> +{
> >> + struct tee_client_device *tee_device;
> >> + struct tee_ioctl_open_session_arg sess_arg;
> >> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> >> + struct tee_rproc *trproc;
> >> + int ret;
> >> +
> >> + /*
> >> + * Test if the device has been probed by the TEE bus. In case of failure, we ignore the
> >> + * reason. The bus could be not yet probed or the service not available in the secure
> >> + * firmware.The assumption in such a case is that the TEE remoteproc is not probed.
> >> + */
> >> + if (!tee_rproc_ctx)
> >> + return ERR_PTR(-EPROBE_DEFER);
> >> +
> >> + trproc = devm_kzalloc(dev, sizeof(*trproc), GFP_KERNEL);
> >> + if (!trproc)
> >> + return ERR_PTR(-ENOMEM);
> >> +
> >> + tee_device = to_tee_client_device(tee_rproc_ctx->dev);
> >> + memset(&sess_arg, 0, sizeof(sess_arg));
> >> +
> >> + memcpy(sess_arg.uuid, tee_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
> >> +
> >> + sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL;
> >> + sess_arg.num_params = 1;
> >> +
> >> + param[0] = (struct tee_param) {
> >> + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> >> + .u.value.a = rproc_id,
> >> + };
> >> +
> >> + ret = tee_client_open_session(tee_rproc_ctx->tee_ctx, &sess_arg, param);
> >> + if (ret < 0 || sess_arg.ret != 0) {
> >> + dev_err(dev, "tee_client_open_session failed, err: %x\n", sess_arg.ret);
> >> + return ERR_PTR(-EINVAL);
> >> + }
> >> +
> >> + trproc->parent = dev;
> >> + trproc->rproc_id = rproc_id;
> >> + trproc->session_id = sess_arg.session;
> >> +
> >> + trproc->rproc = rproc;
> >> + rproc->tee_interface = trproc;
> >> +
> >> + list_add_tail(&trproc->node, &tee_rproc_ctx->sessions);
> >> +
> >> + return trproc;
> >
> > Once this function was called by a client, what prevents a user from unloading
> > the tee_remoteproc module and breaking everything?
>
> Good point! seems better toremove the module build capability
>
I was thinking more along the lines of try_module_get() and module_put() to
avoid bloating the core.
> Thanks,
> Arnaud
>
> >
> >> +}
> >> +EXPORT_SYMBOL_GPL(tee_rproc_register);
> >> +
> >> +int tee_rproc_unregister(struct tee_rproc *trproc)
> >> +{
> >
> > If you pass a struct_rproc instead of a struct tee_rproc there is no need for
> > tee_rproc::rproc, which is only ever used in this function.
> >
> >
> >> + struct rproc *rproc = trproc->rproc;
> >> + int ret;
> >> +
> >> + ret = tee_client_close_session(tee_rproc_ctx->tee_ctx, trproc->session_id);
> >> + if (ret < 0)
> >> + dev_err(trproc->parent, "tee_client_close_session failed, err: %x\n", ret);
> >> +
> >> + list_del(&trproc->node);
> >> + rproc->tee_interface = NULL;
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tee_rproc_unregister);
> >> +
> >> +static int tee_rproc_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> >> +{
> >> + /* Today we support only the OP-TEE, could be extend to other tees */
> >> + return (ver->impl_id == TEE_IMPL_ID_OPTEE);
> >> +}
> >> +
> >> +static int tee_rproc_probe(struct device *dev)
> >> +{
> >> + struct tee_context *tee_ctx;
> >> + int ret;
> >> +
> >> + /* Open context with TEE driver */
> >> + tee_ctx = tee_client_open_context(NULL, tee_rproc_ctx_match, NULL, NULL);
> >> + if (IS_ERR(tee_ctx))
> >> + return PTR_ERR(tee_ctx);
> >> +
> >> + tee_rproc_ctx = devm_kzalloc(dev, sizeof(*tee_ctx), GFP_KERNEL);
> >> + if (!tee_rproc_ctx) {
> >> + ret = -ENOMEM;
> >> + goto err;
> >> + }
> >> +
> >> + tee_rproc_ctx->dev = dev;
> >> + tee_rproc_ctx->tee_ctx = tee_ctx;
> >> + INIT_LIST_HEAD(&tee_rproc_ctx->sessions);
> >> +
> >> + return 0;
> >> +err:
> >> + tee_client_close_context(tee_ctx);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int tee_rproc_remove(struct device *dev)
> >> +{
> >> + struct tee_rproc *entry, *tmp;
> >> +
> >> + list_for_each_entry_safe(entry, tmp, &tee_rproc_ctx->sessions, node) {
> >> + tee_client_close_session(tee_rproc_ctx->tee_ctx, entry->session_id);
> >> + list_del(&entry->node);
> >> + if (entry->rsc_table)
> >> + iounmap(entry->rsc_table);
> >> + kfree(entry);
> >> + }
> >> +
> >> + tee_client_close_context(tee_rproc_ctx->tee_ctx);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +MODULE_DEVICE_TABLE(tee, stm32_tee_rproc_id_table);
> >> +
> >> +static struct tee_client_driver tee_rproc_fw_driver = {
> >> + .id_table = stm32_tee_rproc_id_table,
> >> + .driver = {
> >> + .name = KBUILD_MODNAME,
> >> + .bus = &tee_bus_type,
> >> + .probe = tee_rproc_probe,
> >> + .remove = tee_rproc_remove,
> >> + },
> >> +};
> >> +
> >> +static int __init tee_rproc_fw_mod_init(void)
> >> +{
> >> + return driver_register(&tee_rproc_fw_driver.driver);
> >> +}
> >> +
> >> +static void __exit tee_rproc_fw_mod_exit(void)
> >> +{
> >> + driver_unregister(&tee_rproc_fw_driver.driver);
> >> +}
> >> +
> >> +module_init(tee_rproc_fw_mod_init);
> >> +module_exit(tee_rproc_fw_mod_exit);
> >> +
> >> +MODULE_DESCRIPTION(" TEE remote processor control driver");
> >> +MODULE_LICENSE("GPL");
> >> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> >> index b4795698d8c2..8b678009e481 100644
> >> --- a/include/linux/remoteproc.h
> >> +++ b/include/linux/remoteproc.h
> >> @@ -503,6 +503,8 @@ enum rproc_features {
> >> RPROC_MAX_FEATURES,
> >> };
> >>
> >> +struct tee_rproc;
> >> +
> >> /**
> >> * struct rproc - represents a physical remote processor device
> >> * @node: list node of this rproc object
> >> @@ -545,6 +547,7 @@ enum rproc_features {
> >> * @cdev: character device of the rproc
> >> * @cdev_put_on_release: flag to indicate if remoteproc should be shutdown on @char_dev release
> >> * @features: indicate remoteproc features
> >> + * @tee_interface: pointer to the remoteproc tee context
> >> */
> >> struct rproc {
> >> struct list_head node;
> >> @@ -586,6 +589,7 @@ struct rproc {
> >> struct cdev cdev;
> >> bool cdev_put_on_release;
> >> DECLARE_BITMAP(features, RPROC_MAX_FEATURES);
> >> + struct tee_rproc *tee_interface;
> >> };
> >>
> >> /**
> >> diff --git a/include/linux/tee_remoteproc.h b/include/linux/tee_remoteproc.h
> >> new file mode 100644
> >> index 000000000000..571e47190d02
> >> --- /dev/null
> >> +++ b/include/linux/tee_remoteproc.h
> >> @@ -0,0 +1,112 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * Copyright(c) 2024 STMicroelectronics - All Rights Reserved
> >> + */
> >> +
> >> +#ifndef TEE_REMOTEPROC_H
> >> +#define TEE_REMOTEPROC_H
> >> +
> >> +#include <linux/tee_drv.h>
> >> +#include <linux/firmware.h>
> >> +#include <linux/remoteproc.h>
> >> +
> >> +struct rproc;
> >> +
> >> +/**
> >> + * struct tee_rproc - TEE remoteproc structure
> >> + * @node: Reference in list
> >> + * @rproc: Remoteproc reference
> >> + * @parent: Parent device
> >> + * @rproc_id: Identifier of the target firmware
> >> + * @session_id: TEE session identifier
> >> + * @rsc_table: Resource table virtual address.
> >> + */
> >> +struct tee_rproc {
> >> + struct list_head node;
> >> + struct rproc *rproc;
> >> + struct device *parent;
> >> + u32 rproc_id;
> >> + u32 session_id;
> >> + struct resource_table *rsc_table;
> >> +};
> >> +
> >> +#if IS_REACHABLE(CONFIG_TEE_REMOTEPROC)
> >> +
> >> +struct tee_rproc *tee_rproc_register(struct device *dev, struct rproc *rproc,
> >> + unsigned int rproc_id);
> >> +int tee_rproc_unregister(struct tee_rproc *trproc);
> >> +int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw);
> >> +int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw);
> >> +struct resource_table *tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz);
> >> +struct resource_table *tee_rproc_find_loaded_rsc_table(struct rproc *rproc,
> >> + const struct firmware *fw);
> >> +int tee_rproc_start(struct rproc *rproc);
> >> +int tee_rproc_stop(struct rproc *rproc);
> >> +
> >> +#else
> >> +
> >> +static inline struct tee_rproc *tee_rproc_register(struct device *dev, struct rproc *rproc,
> >> + unsigned int rproc_id)
> >> +{
> >> + return ERR_PTR(-ENODEV);
> >> +}
> >> +
> >> +static int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> >> +{
> >> + /* This shouldn't be possible */
> >> + WARN_ON(1);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static inline int tee_rproc_unregister(struct tee_rproc *trproc)
> >> +{
> >> + /* This shouldn't be possible */
> >> + WARN_ON(1);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static inline int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw)
> >> +{
> >> + /* This shouldn't be possible */
> >> + WARN_ON(1);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static inline int tee_rproc_start(struct rproc *rproc)
> >> +{
> >> + /* This shouldn't be possible */
> >> + WARN_ON(1);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static inline int tee_rproc_stop(struct rproc *rproc)
> >> +{
> >> + /* This shouldn't be possible */
> >> + WARN_ON(1);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static inline struct resource_table *
> >> +tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
> >> +{
> >> + /* This shouldn't be possible */
> >> + WARN_ON(1);
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> +static inline struct resource_table *
> >> +tee_rproc_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
> >> +{
> >> + /* This shouldn't be possible */
> >> + WARN_ON(1);
> >> +
> >> + return NULL;
> >> +}
> >> +#endif /* CONFIG_TEE_REMOTEPROC */
> >> +#endif /* TEE_REMOTEPROC_H */
> >> --
> >> 2.25.1
> >>
^ permalink raw reply
* Re: [PATCH v1 1/8] dt-bindings: clock: add Loongson-2K expand clock index
From: Conor Dooley @ 2024-03-27 16:55 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu,
Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch
In-Reply-To: <CAMpQs4JKYxJQ=+iRW6EqRc4t3wF=eNLVg0Y=943D+X+LmJyG=Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 621 bytes --]
On Wed, Mar 27, 2024 at 07:15:40AM +0600, Binbin Zhou wrote:
> On Wed, Mar 27, 2024 at 12:54 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Tue, Mar 26, 2024 at 05:01:00PM +0800, Binbin Zhou wrote:
> >
> > > -#define LOONGSON2_CLK_END 18
> >
> >
> > > +#define LOONGSON2_CLK_END 35
> >
> > Please just delete this. If you can change it, it is not a binding.
> > Just define it in the driver if it is needed there.
>
> Hi Conor:
>
> Ok, I will drop it in the next version.
With that removed
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: dma: snps,dw-axi-dmac: Add JH8100 support
From: Conor Dooley @ 2024-03-27 16:54 UTC (permalink / raw)
To: ChunHau Tan
Cc: Krzysztof Kozlowski, Eugeniy Paltsev, Vinod Koul, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Leyfoon Tan, JeeHeng Sia,
dmaengine@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <BJSPR01MB0595D132CB5A072A9E61F9C09E34A@BJSPR01MB0595.CHNPR01.prod.partner.outlook.cn>
[-- Attachment #1: Type: text/plain, Size: 2122 bytes --]
On Wed, Mar 27, 2024 at 08:24:01AM +0000, ChunHau Tan wrote:
>
>
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Sent: Wednesday, 27 March, 2024 3:50 PM
> > To: ChunHau Tan <chunhau.tan@starfivetech.com>; Eugeniy Paltsev
> > <Eugeniy.Paltsev@synopsys.com>; Vinod Koul <vkoul@kernel.org>; Rob Herring
> > <robh@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>;
> > Conor Dooley <conor+dt@kernel.org>
> > Cc: Leyfoon Tan <leyfoon.tan@starfivetech.com>; JeeHeng Sia
> > <jeeheng.sia@starfivetech.com>; dmaengine@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v2 1/2] dt-bindings: dma: snps,dw-axi-dmac: Add JH8100
> > support
> >
> > On 27/03/2024 03:51, Tan Chun Hau wrote:
> > > Add support for StarFive JH8100 SoC in Sysnopsys Designware AXI DMA
> > > controller.
> > >
> > > Both JH8100 and JH7110 require reset operation in device probe.
> > > However, JH8100 doesn't need to apply different configuration on
> > > CH_CFG registers.
> >
> > This is a friendly reminder during the review process.
> >
> > It looks like you received a tag and forgot to add it.
> >
> > If you do not know the process, here is a short explanation:
> > Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions,
> > under or above your Signed-off-by tag. Tag is "received", when provided in a
> > message replied to you on the mailing list. Tools like b4 can help here. However,
> > there's no need to repost patches *only* to add the tags. The upstream
> > maintainer will do that for tags received on the version they apply.
> >
> > https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitt
> > ing-patches.rst#L577
> >
> > If a tag was not added on purpose, please state why and what changed.
>
> Hi Krzysztof, thank you very much for the feedback,
> Sorry I overlooked it.
> Do you prefer I resend V2 patch or send a V3 patch to include the Acked-by ?
I resent the tag, you do not need to do anything.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH] of: module: prevent NULL pointer dereference in vsnprintf()
From: Sergey Shtylyov @ 2024-03-27 16:52 UTC (permalink / raw)
To: Rob Herring, Frank Rowand, devicetree, Saravana Kannan; +Cc: lvc-project
In of_modalias(), we can get passed the str and len parameters which would
cause a kernel oops in vsnprintf() since it only allows passing a NULL ptr
when the length is also 0. Also, we need to filter out the negative values
of the len parameter as these will result in a really huge buffer since
snprintf() takes size_t parameter while ours is ssize_t...
Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
The patch is against the for-next branch of Rob Herring's linux-git repo...
drivers/of/module.c | 8 ++++++++
1 file changed, 8 insertions(+)
Index: linux/drivers/of/module.c
===================================================================
--- linux.orig/drivers/of/module.c
+++ linux/drivers/of/module.c
@@ -16,6 +16,14 @@ ssize_t of_modalias(const struct device_
ssize_t csize;
ssize_t tsize;
+ /*
+ * Prevent a kernel oops in vsnprintf() -- it only allows passing a
+ * NULL ptr when the length is also 0. Also filter out the negative
+ * lengths...
+ */
+ if ((len > 0 && !str) || len < 0)
+ return -EINVAL;
+
/* Name & Type */
/* %p eats all alphanum characters, so %c must be used here */
csize = snprintf(str, len, "of:N%pOFn%c%s", np, 'T',
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: crypto: ti,omap-sham: Convert to dtschema
From: Conor Dooley @ 2024-03-27 16:51 UTC (permalink / raw)
To: Animesh Agarwal
Cc: Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-crypto, devicetree, linux-kernel
In-Reply-To: <20240327054911.43093-1-animeshagarwal28@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 275 bytes --]
On Wed, Mar 27, 2024 at 11:19:06AM +0530, Animesh Agarwal wrote:
> Convert the OMAP SoC SHA crypto Module bindings to DT Schema.
>
> Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3] dt-bindings: ata: ahci-da850: Convert to dtschema
From: Conor Dooley @ 2024-03-27 16:45 UTC (permalink / raw)
To: Animesh Agarwal
Cc: Damien Le Moal, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-ide, devicetree, linux-kernel
In-Reply-To: <20240327064354.17384-1-animeshagarwal28@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3136 bytes --]
On Wed, Mar 27, 2024 at 12:13:51PM +0530, Animesh Agarwal wrote:
> Convert the ahci-da850 bindings to DT schema.
>
> Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>
>
> ---
> Changes in v3:
> - Fixed line length issue on line 20
> - Removed unneccessary '|' character
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Cheers,
Conor.
> Changes in v2:
> - Added description for reg property items.
> ---
> .../devicetree/bindings/ata/ahci-da850.txt | 18 ---------
> .../bindings/ata/ti,da850-ahci.yaml | 39 +++++++++++++++++++
> 2 files changed, 39 insertions(+), 18 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt
> create mode 100644 Documentation/devicetree/bindings/ata/ti,da850-ahci.yaml
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-da850.txt b/Documentation/devicetree/bindings/ata/ahci-da850.txt
> deleted file mode 100644
> index 5f8193417725..000000000000
> --- a/Documentation/devicetree/bindings/ata/ahci-da850.txt
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -Device tree binding for the TI DA850 AHCI SATA Controller
> ----------------------------------------------------------
> -
> -Required properties:
> - - compatible: must be "ti,da850-ahci"
> - - reg: physical base addresses and sizes of the two register regions
> - used by the controller: the register map as defined by the
> - AHCI 1.1 standard and the Power Down Control Register (PWRDN)
> - for enabling/disabling the SATA clock receiver
> - - interrupts: interrupt specifier (refer to the interrupt binding)
> -
> -Example:
> -
> - sata: sata@218000 {
> - compatible = "ti,da850-ahci";
> - reg = <0x218000 0x2000>, <0x22c018 0x4>;
> - interrupts = <67>;
> - };
> diff --git a/Documentation/devicetree/bindings/ata/ti,da850-ahci.yaml b/Documentation/devicetree/bindings/ata/ti,da850-ahci.yaml
> new file mode 100644
> index 000000000000..ce13c76bdffb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ti,da850-ahci.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/ti,da850-ahci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI DA850 AHCI SATA Controller
> +
> +maintainers:
> + - Animesh Agarwal <animeshagarwal28@gmail.com>
> +
> +properties:
> + compatible:
> + const: ti,da850-ahci
> +
> + reg:
> + items:
> + - description: Address and size of the register map as defined by the AHCI 1.1 standard.
> + - description:
> + Address and size of Power Down Control Register (PWRDN) for enabling/disabling the SATA clock
> + receiver.
> +
> + interrupts:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + sata@218000 {
> + compatible = "ti,da850-ahci";
> + reg = <0x218000 0x2000>, <0x22c018 0x4>;
> + interrupts = <67>;
> + };
> --
> 2.44.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v1 1/4] dt-bindings: input: Add Himax HX83102J touchscreen
From: Conor Dooley @ 2024-03-27 16:44 UTC (permalink / raw)
To: Allen Lin
Cc: Conor Dooley, Rob Herring, dmitry.torokhov,
krzysztof.kozlowski+dt, conor+dt, jikos, benjamin.tissoires,
linux-input, devicetree, linux-kernel
In-Reply-To: <TY0PR06MB5611AE812B72B349E85118D59E342@TY0PR06MB5611.apcprd06.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 4223 bytes --]
On Wed, Mar 27, 2024 at 03:48:48PM +0800, Allen Lin wrote:
> Conor Dooley <conor@kernel.org> 於 2024年3月27日 週三 上午3:28寫道：
> >
> > On Tue, Mar 26, 2024 at 06:40:28PM +0800, Allen Lin wrote:
> > > Conor Dooley <conor.dooley@microchip.com> 於 2024年3月26日 週二 下午4:48寫道：
> > > >
> > > > On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> > > > > Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> > > > > >
> > > > > > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > > > > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > > > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > > > > > touchscreen controller.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > > > > > >
> > > > > > > > note to self/Krzysztof/Rob:
> > > > > > > > There was a previous attempt at this kind of device. This version looks
> > > > > > > > better but might be incomplete given there's a bunch more properties in
> > > > > > > > that patchset:
> > > > > > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > > > > > >
> > > > > > > Those don't look like properties we want coming back.
> > > > > >
> > > > > > Oh, I don't want most of them coming back either. There are some
> > > > > > supplies in there though that I think we would like to come back, no?
> > > > > > Maybe this particular device doesn't have any supplies, but that doesn't
> > > > > > really seem credible.
> > > > >
> > > > > We will use Firmware-name in Device Tree.
> > > >
> > > > > For power supply settings, because there may be other device using
> > > > > same regulator.
> > > >
> > > > If there are other devices using the same regulator is it more
> > > > important that you document it so that it doesn't get disabled by the
> > > > other users.
> > > >
> > > > > We plan to define it as an optional property for user to control in
> > > > > next release.
> > > >
> > > > I don't see how the regulator would not be required, the device doesn't
> > > > function without power.
> > > >
> > > > Thanks,
> > > > Conor.
> > >
> > > I will set power supply as required.
> > > The description of power supply as below,
> > >
> > > properties:
> > > vccd-supply:
> > > description: A phandle for the regualtor supplying IO power. Should be own
> > > by TPIC only.
> >
> > What does "owned by TPIC" only mean? Why would the vccd supply not be
> > allowed to be shared with another device?
> >
> > > This works for TP digital IO only, main power is
> > > given by display part VSP/VSN power source which is controlled
> > > by lcm driver.
> >
> > What drivers control things doesn't really matter here, we're just
> > describing the hardware. If there's another supply to the controller,
> > then document it too please.
> >
>
> Below is IC power sequence introduction.
> https://github.com/HimaxSoftware/Doc/tree/main/Himax_Chipset_Power_Sequence
>
> TDDI IC, which means Touch and Display Driver is integrated in one IC,
> So some power supplies will be controlled by Display driver.
If someone was to turn off the supplies, would the touch component stop
working? The document says that these supplies must be turned on before
the touch supplies, so I'm going to assume that both are needed for the
device to function.
> In yaml Document, can we just list power supplies controlled by touch driver?
If the touch part of this device needs the supplies to function, then
you need to mention them, regardless of where they're controlled. All we
are doing in the binding is describing the hardware. What drivers do
what depends entirely on what software you're using.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] phy: rockchip: emmc: Enable pulldown for strobe line
From: Folker Schwesinger @ 2024-03-27 16:21 UTC (permalink / raw)
To: Dragan Simic, Conor Dooley
Cc: Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, Chris Ruehl,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Christopher Obbard, Alban Browaeys, Doug Anderson, Brian Norris,
Jensen Huang, linux-phy, linux-arm-kernel, linux-rockchip,
linux-kernel, devicetree
In-Reply-To: <436f78a981ecba441a0636912ddd1cf2@manjaro.org>
[-- Attachment #1: Type: text/plain, Size: 3995 bytes --]
Hi Conor and Dragan,
thanks for your feedback!
On Tue Mar 26, 2024 at 8:55 PM CET, Dragan Simic wrote:
> On 2024-03-26 20:46, Conor Dooley wrote:
> > On Tue, Mar 26, 2024 at 07:54:35PM +0100, Folker Schwesinger via B4
> > Relay wrote:
> >> From: Folker Schwesinger <dev@folker-schwesinger.de>
> >>
> >> Restore the behavior of the Rockchip kernel that undconditionally
> >> enables the internal strobe pulldown.
> >
> > What do you mean "restore the behaviour of the rockchip kernel"? Did
> > mainline behave the same as the rockchip kernel previously? If not,
> > using "restore" here is misleading. "Unconditionally" is also
> > incorrect,
> > because you have a property that disables it.
Apologizes for the misleading commit message. Prior to 5.11 the Linux
kernel did not touch the pull-down registers. However, it seems the
register's (factory?) default was set to enable the pull-down. As it
was mentioned elsewhere that was the configuration recommended by
Rockchip. The 4.4 vendor (Rockchip) kernel reflects that by enabling the
pull-down in its kernel.
Of course, this has nothing to do with the Linux kernel, so "restore"
was a bad choice here.
I previously had split the driver patch into two separate patches, one
for changing the default (unconditionally at that point), the other for
adding the disable property. As both changes were minimal I decided to
squash the commits. I updated the cover letter, but forgot to update the
commit message. Sorry.
> >> Fixes: 8b5c2b45b8f0 ("phy: rockchip: set pulldown for strobe line in
> >> dts")
> >> Signed-off-by: Folker Schwesinger <dev@folker-schwesinger.de>
> >> ---
> >> drivers/phy/rockchip/phy-rockchip-emmc.c | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/phy/rockchip/phy-rockchip-emmc.c
> >> b/drivers/phy/rockchip/phy-rockchip-emmc.c
> >> index 20023f6eb994..6e637f3e1b19 100644
> >> --- a/drivers/phy/rockchip/phy-rockchip-emmc.c
> >> +++ b/drivers/phy/rockchip/phy-rockchip-emmc.c
> >> @@ -376,14 +376,14 @@ static int rockchip_emmc_phy_probe(struct
> >> platform_device *pdev)
> >> rk_phy->reg_offset = reg_offset;
> >> rk_phy->reg_base = grf;
> >> rk_phy->drive_impedance = PHYCTRL_DR_50OHM;
> >> - rk_phy->enable_strobe_pulldown = PHYCTRL_REN_STRB_DISABLE;
> >> + rk_phy->enable_strobe_pulldown = PHYCTRL_REN_STRB_ENABLE;
> >> rk_phy->output_tapdelay_select = PHYCTRL_OTAPDLYSEL_DEFAULT;
> >>
> >> if (!of_property_read_u32(dev->of_node, "drive-impedance-ohm",
> >> &val))
> >> rk_phy->drive_impedance = convert_drive_impedance_ohm(pdev, val);
> >>
> >> - if (of_property_read_bool(dev->of_node,
> >> "rockchip,enable-strobe-pulldown"))
> >> - rk_phy->enable_strobe_pulldown = PHYCTRL_REN_STRB_ENABLE;
> >> + if (of_property_read_bool(dev->of_node,
> >> "rockchip,disable-strobe-pulldown"))
> >> + rk_phy->enable_strobe_pulldown = PHYCTRL_REN_STRB_DISABLE;
> >
> > Unfortunately you cannot do this.
> > Previously no property at all meant disabled and a property was
> > required
> > to enable it. With this change the absence of a property means that it
> > will be enabled.
> > An old devicetree is that wanted this to be disabled would have no
> > property and will now end up with it enabled. This is an ABI break and
> > is
> > clearly not backwards compatible, that's a NAK unless it is
> > demonstrable
> > that noone actually wants to disable it at all.
> >
>
> Moreover, as I already explained some time ago, [1] some boards and
> devices are unfortunately miswired, and we don't want to enable the
> DATA STROBE pull-down on such boards.
>
> [1]
> https://lore.kernel.org/linux-rockchip/ca5b7cad01f645c7c559ab26a8db8085@manjaro.org/#t
>
> > If this patch fixes a problem on a board that you have, I would suggest
> > that you add the property to enable it, as the binding tells you to.
I agree, I'll post the patches later.
Best regards
Folker
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]
^ 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