Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 2/3] firmware: raspberrypi: Use correct device for DMA mappings
From: Laurent Pinchart @ 2024-03-26 19:58 UTC (permalink / raw)
  To: devicetree, linux-rpi-kernel, linux-arm-kernel
  Cc: Dave Stevenson, Naushir Patuck,
	Broadcom internal kernel review list, Conor Dooley,
	Florian Fainelli, Krzysztof Kozlowski, Nicolas Saenz Julienne,
	Ray Jui, Rob Herring, Scott Branden, Stefan Wahren
In-Reply-To: <20240326195807.15163-1-laurent.pinchart@ideasonboard.com>

The buffer used to transfer data over the mailbox interface is mapped
using the client's device. This is incorrect, as the device performing
the DMA transfer is the mailbox itself. Fix it by using the mailbox
controller device instead.

This requires including the mailbox_controller.h header to dereference
the mbox_chan and mbox_controller structures. The header is not meant to
be included by clients. This could be fixed by extending the client API
with a function to access the controller's device.

Fixes: 4e3d60656a72 ("ARM: bcm2835: Add the Raspberry Pi firmware driver")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/firmware/raspberrypi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 322aada20f74..ac34876a97f8 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -9,6 +9,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/kref.h>
 #include <linux/mailbox_client.h>
+#include <linux/mailbox_controller.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
@@ -97,8 +98,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
 	if (size & 3)
 		return -EINVAL;
 
-	buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
-				 GFP_ATOMIC);
+	buf = dma_alloc_coherent(fw->chan->mbox->dev, PAGE_ALIGN(size),
+				 &bus_addr, GFP_ATOMIC);
 	if (!buf)
 		return -ENOMEM;
 
@@ -126,7 +127,7 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
 		ret = -EINVAL;
 	}
 
-	dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
+	dma_free_coherent(fw->chan->mbox->dev, PAGE_ALIGN(size), buf, bus_addr);
 
 	return ret;
 }
-- 
Regards,

Laurent Pinchart


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 1/3] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node
From: Laurent Pinchart @ 2024-03-26 19:58 UTC (permalink / raw)
  To: devicetree, linux-rpi-kernel, linux-arm-kernel
  Cc: Dave Stevenson, Naushir Patuck, Bartosz Golaszewski,
	Broadcom internal kernel review list, Conor Dooley,
	Florian Fainelli, Krzysztof Kozlowski, Linus Walleij, Ray Jui,
	Rob Herring, Scott Branden, Stefan Wahren
In-Reply-To: <20240326195807.15163-1-laurent.pinchart@ideasonboard.com>

Unlike the other child nodes of the raspberrypi,bcm2835-firmware device,
the gpio child is documented in a legacy text-based binding in
gpio/raspberrypi,firmware-gpio.txt. This causes DT validation failures:

arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: 'gpio' does not match any of the regexes: 'pinctrl-[0-9]+'
        from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#

Convert the binding to YAML and move it to
raspberrypi,bcm2835-firmware.yaml.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes since v1:

- Add minItems for gpio-line-names
---
 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 30 +++++++++++++++++++
 .../gpio/raspberrypi,firmware-gpio.txt        | 30 -------------------
 2 files changed, 30 insertions(+), 30 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt

diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index 39e3c248f5b7..1f84407a73e4 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -46,6 +46,30 @@ properties:
       - compatible
       - "#clock-cells"
 
+  gpio:
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        const: raspberrypi,firmware-gpio
+
+      gpio-controller: true
+
+      "#gpio-cells":
+        const: 2
+        description:
+          The first cell is the pin number, and the second cell is used to
+          specify the gpio polarity (GPIO_ACTIVE_HIGH or GPIO_ACTIVE_LOW).
+
+      gpio-line-names:
+        minItems: 8
+
+    required:
+      - compatible
+      - gpio-controller
+      - "#gpio-cells"
+
   reset:
     type: object
     additionalProperties: false
@@ -96,6 +120,12 @@ examples:
             #clock-cells = <1>;
         };
 
+        expgpio: gpio {
+            compatible = "raspberrypi,firmware-gpio";
+            gpio-controller;
+            #gpio-cells = <2>;
+        };
+
         reset: reset {
             compatible = "raspberrypi,firmware-reset";
             #reset-cells = <1>;
diff --git a/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
deleted file mode 100644
index ce97265e23ba..000000000000
--- a/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Raspberry Pi GPIO expander
-
-The Raspberry Pi 3 GPIO expander is controlled by the VC4 firmware. The
-firmware exposes a mailbox interface that allows the ARM core to control the
-GPIO lines on the expander.
-
-The Raspberry Pi GPIO expander node must be a child node of the Raspberry Pi
-firmware node.
-
-Required properties:
-
-- compatible : Should be "raspberrypi,firmware-gpio"
-- gpio-controller : Marks the device node as a gpio controller
-- #gpio-cells : Should be two.  The first cell is the pin number, and
-  the second cell is used to specify the gpio polarity:
-  0 = active high
-  1 = active low
-
-Example:
-
-firmware: firmware-rpi {
-	compatible = "raspberrypi,bcm2835-firmware";
-	mboxes = <&mailbox>;
-
-	expgpio: gpio {
-		 compatible = "raspberrypi,firmware-gpio";
-		 gpio-controller;
-		 #gpio-cells = <2>;
-	 };
-};
-- 
Regards,

Laurent Pinchart


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 0/3] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Drive-by fixes
From: Laurent Pinchart @ 2024-03-26 19:58 UTC (permalink / raw)
  To: devicetree, linux-rpi-kernel, linux-arm-kernel
  Cc: Dave Stevenson, Naushir Patuck, Bartosz Golaszewski,
	Broadcom internal kernel review list, Conor Dooley,
	Florian Fainelli, Krzysztof Kozlowski, Linus Walleij,
	Nicolas Saenz Julienne, Ray Jui, Rob Herring, Scott Branden,
	Stefan Wahren

Hello,

This small series includes a few drive-by fixes for DT validation
errors.

The first patch has been posted previously in v1 ([1], and now addresses
a small review comment. I think it's good to go.

The next two patches address the same issue as "[PATCH 1/2] dt-bindings:
arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties" ([2]),
but this time with a (hopefully) correct approach. Patch 2/3 starts by
fixing the raspberrypi-bcm2835-firmware driver, removing the need for DT
properties that are specified in bcm2835-rpi.dtsi but not documented in
the corresponding bindings. Patch 3/3 can then drop those properties,
getting rid of the warnings.

[1] https://lore.kernel.org/linux-arm-kernel/20240326004902.17054-3-laurent.pinchart@ideasonboard.com/
[2] https://lore.kernel.org/linux-arm-kernel/20240326004902.17054-2-laurent.pinchart@ideasonboard.com/

Laurent Pinchart (3):
  dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child
    node
  firmware: raspberrypi: Use correct device for DMA mappings
  ARM: dts: bcm283x: Drop unneeded properties in the bcm2835-firmware
    node

 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 30 +++++++++++++++++++
 .../gpio/raspberrypi,firmware-gpio.txt        | 30 -------------------
 arch/arm/boot/dts/broadcom/bcm2835-rpi.dtsi   |  4 ---
 drivers/firmware/raspberrypi.c                |  7 +++--
 4 files changed, 34 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt

-- 
Regards,

Laurent Pinchart


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH RESEND v6 0/5] spmi: pmic-arb: Add support for multiple buses
From: Abel Vesa @ 2024-03-26 19:57 UTC (permalink / raw)
  To: Rob Herring
  Cc: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
	Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
	Krzysztof Kozlowski, Conor Dooley, Srini Kandagatla, Johan Hovold,
	linux-kernel, linux-arm-kernel, linux-arm-msm, linux-mediatek,
	devicetree
In-Reply-To: <20240326193203.GA3252922-robh@kernel.org>

On 24-03-26 14:32:03, Rob Herring wrote:
> On Tue, Mar 26, 2024 at 06:28:15PM +0200, Abel Vesa wrote:
> > This RFC prepares for and adds support for 2 buses, which is supported
> > in HW starting with version 7. Until now, none of the currently
> > supported platforms in upstream have used the second bus. The X1E80100
> > platform, on the other hand, needs the second bus for the USB2.0 to work
> > as there are 3 SMB2360 PMICs which provide eUSB2 repeaters and they are
> > all found on the second bus.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> > Changes in v6:
> > - Changed the compatible to platform specific (X1E80100) along with the
> >   schema. Fixed the spmi buses unit addresses and added the empty ranges
> >   property. Added missing properties to the spmi buses and the
> >   "unevaluatedProperties: false".
> > - Deprecated the "qcom,bus-id" in the legacy schema.
> > - Changed the driver to check for legacy compatible first
> > - Link to v5: https://lore.kernel.org/r/20240221-spmi-multi-master-support-v5-0-3255ca413a0b@linaro.org
> 
> Where are Krzysztof's Reviewed-by tags?

Urgh, did "b4 send --resend" which only sent the v5 as it was before the
Reviewed-by tags have been picked up.

My bad.

I'll send a v6 with the tags appended.

> 
> Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: rockchip: quartzpro64: Enable the GPU
From: Dragan Simic @ 2024-03-26 19:57 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: linux-rockchip, heiko, linux-arm-kernel, devicetree, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, boris.brezillon, linux-kernel,
	kernel
In-Reply-To: <jbyqey5y5ngr7mkrrmdxrwyw5ogd7rq56af6mrmhsckboanvyp@tcaav2tridos>

Hello Sebastian,

On 2024-03-26 20:54, Sebastian Reichel wrote:
> On Mon, Mar 25, 2024 at 05:19:04PM +0100, Dragan Simic wrote:
>> Following the approach used to enable the Mali GPU on the rk3588-evb1, 
>> [1]
>> do the same for the Pine64 QuartzPro64, which uses nearly identical 
>> hardware
>> design as the RK3588 EVB1.
>> 
>> The slight disadvantage is that the regulator coupling logic requires 
>> the
>> regulators to be always on, which is also noted in the comments.  This 
>> is
>> obviously something to be improved at some point in the future, but 
>> should
>> be fine for now, especially because the QuartzPro64 isn't a 
>> battery-powered
>> board, so low power consumption isn't paramount.
>> 
>> [1] 
>> https://lore.kernel.org/linux-rockchip/20240325153850.189128-5-sebastian.reichel@collabora.com/
>> 
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
> 
> FWIW
> 
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Great, thanks for your review!

>>  .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 14 
>> ++++++++++++++
>>  1 file changed, 14 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts 
>> b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
>> index 67414d72e2b6..68d432c61ea5 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
>> @@ -285,6 +285,12 @@ &gmac0_rgmii_clk
>>  	status = "okay";
>>  };
>> 
>> +&gpu {
>> +	mali-supply = <&vdd_gpu_s0>;
>> +	sram-supply = <&vdd_gpu_mem_s0>;
>> +	status = "okay";
>> +};
>> +
>>  &i2c2 {
>>  	status = "okay";
>> 
>> @@ -491,11 +497,15 @@ rk806_dvs3_null: dvs3-null-pins {
>>  		regulators {
>>  			vdd_gpu_s0: dcdc-reg1 {
>>  				regulator-name = "vdd_gpu_s0";
>> +				/* regulator coupling requires always-on */
>> +				regulator-always-on;
>>  				regulator-boot-on;
>>  				regulator-enable-ramp-delay = <400>;
>>  				regulator-min-microvolt = <550000>;
>>  				regulator-max-microvolt = <950000>;
>>  				regulator-ramp-delay = <12500>;
>> +				regulator-coupled-with = <&vdd_gpu_mem_s0>;
>> +				regulator-coupled-max-spread = <10000>;
>> 
>>  				regulator-state-mem {
>>  					regulator-off-in-suspend;
>> @@ -545,11 +555,15 @@ regulator-state-mem {
>> 
>>  			vdd_gpu_mem_s0: dcdc-reg5 {
>>  				regulator-name = "vdd_gpu_mem_s0";
>> +				/* regulator coupling requires always-on */
>> +				regulator-always-on;
>>  				regulator-boot-on;
>>  				regulator-enable-ramp-delay = <400>;
>>  				regulator-min-microvolt = <675000>;
>>  				regulator-max-microvolt = <950000>;
>>  				regulator-ramp-delay = <12500>;
>> +				regulator-coupled-with = <&vdd_gpu_s0>;
>> +				regulator-coupled-max-spread = <10000>;
>> 
>>  				regulator-state-mem {
>>  					regulator-off-in-suspend;
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/3] phy: rockchip: emmc: Enable pulldown for strobe line
From: Dragan Simic @ 2024-03-26 19:55 UTC (permalink / raw)
  To: Conor Dooley
  Cc: dev, 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: <20240326-tactical-onlooker-3df8d2352dc2@spud>

Hello Conor and Folker,

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.
> 
>> As the DT property rockchip,enable-strobe-pulldown is obsolete now,
>> replace it with a property to disable the internal pulldown.
>> 
>> This fixes I/O errors observed on various Rock Pi 4 and NanoPi4 series
>> boards with some eMMC modules. Other boards may also be affected.
>> 
>> An example of these errors is as follows:
>> 
>> [  290.060817] mmc1: running CQE recovery
>> [  290.061337] blk_update_request: I/O error, dev mmcblk1, sector 
>> 1411072 op 0x1:(WRITE) flags 0x800 phys_seg 36 prio class 0
>> [  290.061370] EXT4-fs warning (device mmcblk1p1): ext4_end_bio:348: 
>> I/O error 10 writing to inode 29547 starting block 176466)
>> [  290.061484] Buffer I/O error on device mmcblk1p1, logical block 
>> 172288
>> 
>> 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.
> 
> Thanks,
> Conor.
> 
>>  	if (!of_property_read_u32(dev->of_node, 
>> "rockchip,output-tapdelay-select", &val)) {
>>  		if (val <= PHYCTRL_OTAPDLYSEL_MAXVALUE)
>> 
>> --
>> 2.44.0
>> 
>> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: rockchip: quartzpro64: Enable the GPU
From: Sebastian Reichel @ 2024-03-26 19:54 UTC (permalink / raw)
  To: Dragan Simic
  Cc: linux-rockchip, heiko, linux-arm-kernel, devicetree, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, boris.brezillon, linux-kernel,
	kernel
In-Reply-To: <0f3759ee390f245dac447bbee038445ddfecbec0.1711383286.git.dsimic@manjaro.org>


[-- Attachment #1.1: Type: text/plain, Size: 2699 bytes --]

Hi,

On Mon, Mar 25, 2024 at 05:19:04PM +0100, Dragan Simic wrote:
> Following the approach used to enable the Mali GPU on the rk3588-evb1, [1]
> do the same for the Pine64 QuartzPro64, which uses nearly identical hardware
> design as the RK3588 EVB1.
> 
> The slight disadvantage is that the regulator coupling logic requires the
> regulators to be always on, which is also noted in the comments.  This is
> obviously something to be improved at some point in the future, but should
> be fine for now, especially because the QuartzPro64 isn't a battery-powered
> board, so low power consumption isn't paramount.
> 
> [1] https://lore.kernel.org/linux-rockchip/20240325153850.189128-5-sebastian.reichel@collabora.com/
> 
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---

FWIW

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
> index 67414d72e2b6..68d432c61ea5 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
> @@ -285,6 +285,12 @@ &gmac0_rgmii_clk
>  	status = "okay";
>  };
>  
> +&gpu {
> +	mali-supply = <&vdd_gpu_s0>;
> +	sram-supply = <&vdd_gpu_mem_s0>;
> +	status = "okay";
> +};
> +
>  &i2c2 {
>  	status = "okay";
>  
> @@ -491,11 +497,15 @@ rk806_dvs3_null: dvs3-null-pins {
>  		regulators {
>  			vdd_gpu_s0: dcdc-reg1 {
>  				regulator-name = "vdd_gpu_s0";
> +				/* regulator coupling requires always-on */
> +				regulator-always-on;
>  				regulator-boot-on;
>  				regulator-enable-ramp-delay = <400>;
>  				regulator-min-microvolt = <550000>;
>  				regulator-max-microvolt = <950000>;
>  				regulator-ramp-delay = <12500>;
> +				regulator-coupled-with = <&vdd_gpu_mem_s0>;
> +				regulator-coupled-max-spread = <10000>;
>  
>  				regulator-state-mem {
>  					regulator-off-in-suspend;
> @@ -545,11 +555,15 @@ regulator-state-mem {
>  
>  			vdd_gpu_mem_s0: dcdc-reg5 {
>  				regulator-name = "vdd_gpu_mem_s0";
> +				/* regulator coupling requires always-on */
> +				regulator-always-on;
>  				regulator-boot-on;
>  				regulator-enable-ramp-delay = <400>;
>  				regulator-min-microvolt = <675000>;
>  				regulator-max-microvolt = <950000>;
>  				regulator-ramp-delay = <12500>;
> +				regulator-coupled-with = <&vdd_gpu_s0>;
> +				regulator-coupled-max-spread = <10000>;
>  
>  				regulator-state-mem {
>  					regulator-off-in-suspend;

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/3] phy: rockchip: emmc: Enable pulldown for strobe line
From: Conor Dooley @ 2024-03-26 19:46 UTC (permalink / raw)
  To: dev
  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: <20240326-rk-default-enable-strobe-pulldown-v1-1-f410c71605c0@folker-schwesinger.de>


[-- Attachment #1.1: Type: text/plain, Size: 3387 bytes --]

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.

> As the DT property rockchip,enable-strobe-pulldown is obsolete now,
> replace it with a property to disable the internal pulldown.
> 
> This fixes I/O errors observed on various Rock Pi 4 and NanoPi4 series
> boards with some eMMC modules. Other boards may also be affected.
> 
> An example of these errors is as follows:
> 
> [  290.060817] mmc1: running CQE recovery
> [  290.061337] blk_update_request: I/O error, dev mmcblk1, sector 1411072 op 0x1:(WRITE) flags 0x800 phys_seg 36 prio class 0
> [  290.061370] EXT4-fs warning (device mmcblk1p1): ext4_end_bio:348: I/O error 10 writing to inode 29547 starting block 176466)
> [  290.061484] Buffer I/O error on device mmcblk1p1, logical block 172288
> 
> 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.

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.

Thanks,
Conor.

>  	if (!of_property_read_u32(dev->of_node, "rockchip,output-tapdelay-select", &val)) {
>  		if (val <= PHYCTRL_OTAPDLYSEL_MAXVALUE)
> 
> -- 
> 2.44.0
> 
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 2/3] devicetree: phy: rockchip-emmc: Document changed strobe-pulldown property
From: Conor Dooley @ 2024-03-26 19:34 UTC (permalink / raw)
  To: dev
  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: <20240326-rk-default-enable-strobe-pulldown-v1-2-f410c71605c0@folker-schwesinger.de>


[-- Attachment #1.1: Type: text/plain, Size: 2010 bytes --]

On Tue, Mar 26, 2024 at 07:54:36PM +0100, Folker Schwesinger via B4 Relay wrote:
> From: Folker Schwesinger <dev@folker-schwesinger.de>

The prefix is "dt-bindings" not "devicetree" FYI.

> 
> Document the changes regarding the optional strobe-pulldown property.
> These changes are necessary as the default behavior of the driver was
> restored to the Rockchip kernel behavior of enabling the internal
> pulldown by default.

I don't think this is a valid justification, but it'll be easier for me
to explain this on the driver patch.

Thanks,
Conor.

> 
> Fixes: f34e43f12382 ("devicetree: phy: rockchip-emmc: pulldown property")
> Signed-off-by: Folker Schwesinger <dev@folker-schwesinger.de>
> ---
>  Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
> index 57d28c0d5696..10c05437f7ab 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
> @@ -16,8 +16,8 @@ Optional properties:
>   - drive-impedance-ohm: Specifies the drive impedance in Ohm.
>                          Possible values are 33, 40, 50, 66 and 100.
>                          If not set, the default value of 50 will be applied.
> - - rockchip,enable-strobe-pulldown: Enable internal pull-down for the strobe
> -                                    line.  If not set, pull-down is not used.
> + - rockchip,disable-strobe-pulldown: Disable internal pull-down for the strobe
> +                                     line.  If not set, pull-down is used.
>   - rockchip,output-tapdelay-select: Specifies the phyctrl_otapdlysec register.
>                                      If not set, the register defaults to 0x4.
>                                      Maximum value 0xf.
> 
> -- 
> 2.44.0
> 
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 0/4] arm64: dts: rockchip: add wolfvision pf5 mainboard
From: Heiko Stübner @ 2024-03-26 19:33 UTC (permalink / raw)
  To: Michael Riesch, Rob Herring
  Cc: linux-kernel, linux-arm-kernel, linux-rockchip,
	Krzysztof Kozlowski, devicetree, Conor Dooley
In-Reply-To: <171148006579.3222626.4177463381080253015.robh@kernel.org>

Am Dienstag, 26. März 2024, 20:11:58 CET schrieb Rob Herring:
> 
> On Mon, 25 Mar 2024 15:22:30 +0100, Michael Riesch wrote:
> > Habidere,
> > 
> > This series adds the device tree for the WolfVision PF5 mainboard, which
> > serves as base for recent WolfVision products. It features the Rockchip
> > RK3568 and can be extended with several different extension boards.
> > 
> > The WolfVision PF5 IO Expander is one example of such an extension board.
> > The corresponding device tree overlay is also included in this series.
> > 
> > May this be the beginning of a beautiful friendship :-)
> > 
> > Looking forward to your comments!
> > 
> > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> > ---
> > Michael Riesch (4):
> >       dt-bindings: add wolfvision vendor prefix
> >       dt-bindings: arm: rockchip: add wolfvision pf5 mainboard
> >       arm64: dts: rockchip: add wolfvision pf5 mainboard
> >       arm64: dts: rockchip: add wolfvision pf5 io expander board
> > 
> >  .../devicetree/bindings/arm/rockchip.yaml          |   5 +
> >  .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
> >  arch/arm64/boot/dts/rockchip/Makefile              |   2 +
> >  .../rk3568-wolfvision-pf5-io-expander.dtso         | 137 ++++++
> >  .../boot/dts/rockchip/rk3568-wolfvision-pf5.dts    | 528 +++++++++++++++++++++
> >  5 files changed, 674 insertions(+)
> > ---
> > base-commit: 4cece764965020c22cff7665b18a012006359095
> > change-id: 20240325-feature-wolfvision-pf5-5c1924c0389c
> > 
> > Best regards,
> > --
> > Michael Riesch <michael.riesch@wolfvision.net>
> > 
> > 
> > 
> 
> 
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>   pip3 install dtschema --upgrade
> 
> 
> New warnings running 'make CHECK_DTBS=y rockchip/rk3568-wolfvision-pf5.dtb' for 20240325-feature-wolfvision-pf5-v1-0-5725445f792a@wolfvision.net:
> 
> arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5.dtb: hdmi@fe0a0000: Unevaluated properties are not allowed ('#sound-dai-cells' was unexpected)
> 	from schema $id: http://devicetree.org/schemas/display/rockchip/rockchip,dw-hdmi.yaml#

just for the record, this is not the fault of the Wolfvision board,
but caused by an undocumented property in the core hdmi node.

I've prepared a fix for the binding in [0], but as Krzysztof noted,
this patch needs a v2 with a change.


Heiko



[0] https://lore.kernel.org/dri-devel/20240326172801.1163200-1-heiko@sntech.de/



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Mark Brown @ 2024-03-26 19:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <ZgMf5eISwE2P_1tN@smile.fi.intel.com>


[-- Attachment #1.1: Type: text/plain, Size: 685 bytes --]

On Tue, Mar 26, 2024 at 09:20:05PM +0200, Andy Shevchenko wrote:

> For my knowledge there is none of the ACPI-based platform where CONFIG_ACPI
> needs to be 'n' while having the real device (as per ACPI ID table) to be on.
> That's why I answered purely from the compilation point of view.

I don't understand the relevance of that, and frankly can't make much
sense of it.

> Personally I see that dependency more confusing than hinting about anything.

When you don't have a dependency in Kconfig then people get offered the
device even if it is impossible for it to be useful on their platform.
The purpose of any || COMPILE_TEST dependency is to improve the
usability of Kconfig.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 4/4] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
From: Arnaud POULIQUEN @ 2024-03-26 19:31 UTC (permalink / raw)
  To: Mathieu Poirier
  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: <ZgGrnkcebcIQQic6@p14s>



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.

For instance we can have a system with A DSP and a modem. We would use the same
TEE service, but
the TEE driver will probably be different, same for the signature key.
In such case the proc ID allows to identify the the processor you want to address.


> 
>> +#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
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH RESEND v6 0/5] spmi: pmic-arb: Add support for multiple buses
From: Rob Herring @ 2024-03-26 19:32 UTC (permalink / raw)
  To: Abel Vesa
  Cc: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
	Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
	Krzysztof Kozlowski, Conor Dooley, Srini Kandagatla, Johan Hovold,
	linux-kernel, linux-arm-kernel, linux-arm-msm, linux-mediatek,
	devicetree
In-Reply-To: <20240326-spmi-multi-master-support-v6-0-1c87d8306c5b@linaro.org>

On Tue, Mar 26, 2024 at 06:28:15PM +0200, Abel Vesa wrote:
> This RFC prepares for and adds support for 2 buses, which is supported
> in HW starting with version 7. Until now, none of the currently
> supported platforms in upstream have used the second bus. The X1E80100
> platform, on the other hand, needs the second bus for the USB2.0 to work
> as there are 3 SMB2360 PMICs which provide eUSB2 repeaters and they are
> all found on the second bus.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> Changes in v6:
> - Changed the compatible to platform specific (X1E80100) along with the
>   schema. Fixed the spmi buses unit addresses and added the empty ranges
>   property. Added missing properties to the spmi buses and the
>   "unevaluatedProperties: false".
> - Deprecated the "qcom,bus-id" in the legacy schema.
> - Changed the driver to check for legacy compatible first
> - Link to v5: https://lore.kernel.org/r/20240221-spmi-multi-master-support-v5-0-3255ca413a0b@linaro.org

Where are Krzysztof's Reviewed-by tags?

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] dt-bindings: display: rockchip: add missing #sound-dai-cells to dw-hdmi
From: Heiko Stübner @ 2024-03-26 19:29 UTC (permalink / raw)
  To: hjc, andy.yan, robh, krzysztof.kozlowski+dt, conor+dt,
	Krzysztof Kozlowski
  Cc: dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel
In-Reply-To: <5fb55234-4afd-4e6a-9763-4d2e344e0ce6@linaro.org>

Am Dienstag, 26. März 2024, 18:50:37 CET schrieb Krzysztof Kozlowski:
> On 26/03/2024 18:50, Krzysztof Kozlowski wrote:
> > On 26/03/2024 18:28, Heiko Stuebner wrote:
> >> The #sound-dai-cells DT property is required to describe link between
> >> the HDMI IP block and the SoC's audio subsystem.
> >>
> >> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >> ---
> >>  .../devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> >> index af638b6c0d21..3768df80ca7a 100644
> >> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> >> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> >> @@ -124,6 +124,9 @@ properties:
> >>      description:
> >>        phandle to the GRF to mux vopl/vopb.
> >>  
> >> +  "#sound-dai-cells":
> >> +    const: 0
> >> +
> > 
> > Then you miss $ref in allOf to /schemas/sound/dai-common.yaml
> 
> I meant, except your change you should add also above $ref.

sorry about that, will fix that.

Thanks for the pointer
Heiko




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: rockchip: quartzpro64: Enable the GPU
From: Dragan Simic @ 2024-03-26 19:27 UTC (permalink / raw)
  To: Rob Herring
  Cc: conor+dt, kernel, devicetree, sebastian.reichel, heiko,
	linux-arm-kernel, boris.brezillon, linux-kernel, linux-rockchip,
	robh+dt, krzysztof.kozlowski+dt
In-Reply-To: <171148006524.3222394.1157008244152468379.robh@kernel.org>

On 2024-03-26 20:11, Rob Herring wrote:
> On Mon, 25 Mar 2024 17:19:04 +0100, Dragan Simic wrote:
>> Following the approach used to enable the Mali GPU on the rk3588-evb1, 
>> [1]
>> do the same for the Pine64 QuartzPro64, which uses nearly identical 
>> hardware
>> design as the RK3588 EVB1.
>> 
>> The slight disadvantage is that the regulator coupling logic requires 
>> the
>> regulators to be always on, which is also noted in the comments.  This 
>> is
>> obviously something to be improved at some point in the future, but 
>> should
>> be fine for now, especially because the QuartzPro64 isn't a 
>> battery-powered
>> board, so low power consumption isn't paramount.
>> 
>> [1] 
>> https://lore.kernel.org/linux-rockchip/20240325153850.189128-5-sebastian.reichel@collabora.com/
>> 
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>  .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 14 
>> ++++++++++++++
>>  1 file changed, 14 insertions(+)
> 
> My bot found new DTB warnings on the .dts files added or changed in 
> this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the 
> warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to 
> reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>   pip3 install dtschema --upgrade
> 
> 
> New warnings running 'make CHECK_DTBS=y
> rockchip/rk3588-quartzpro64.dtb' for
> 0f3759ee390f245dac447bbee038445ddfecbec0.1711383286.git.dsimic@manjaro.org:
> 
> Error: arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts:288.1-5
> Label or path gpu not found
> FATAL ERROR: Syntax error parsing input tree
> make[3]: *** [scripts/Makefile.lib:427:
> arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dtb] Error 1
> make[2]: *** [scripts/Makefile.build:485: arch/arm64/boot/dts/rockchip] 
> Error 2
> make[2]: Target 'arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dtb'
> not remade because of errors.
> make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1387:
> rockchip/rk3588-quartzpro64.dtb] Error 2
> make: *** [Makefile:240: __sub-make] Error 2
> make: Target 'rockchip/rk3588-quartzpro64.dtb' not remade because of 
> errors.

This error can be safely ignored, because this patch is supposed
to be applied together with a recent patch series, [1] or in more
detail, after one of the patches in that series [2] that adds the
missing "gpu" node.

[1] 
https://lore.kernel.org/linux-rockchip/20240326165232.73585-1-sebastian.reichel@collabora.com/T/#u
[2] 
https://lore.kernel.org/linux-rockchip/20240326165232.73585-3-sebastian.reichel@collabora.com/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: arm64: marvell: add solidrun cn9130 clearfog boards
From: Josua Mayer @ 2024-03-26 19:26 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
	Sebastian Hesselbarth, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Yazan Shhady, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <b086a02c-624f-4225-a9fb-164095742ce9@linaro.org>

Am 26.03.24 um 07:41 schrieb Krzysztof Kozlowski:
> On 25/03/2024 21:12, Josua Mayer wrote:
>> Am 25.03.24 um 20:34 schrieb Krzysztof Kozlowski:
>>> On 22/03/2024 11:08, Josua Mayer wrote:
>>>> Am 21.03.24 um 22:47 schrieb Josua Mayer:
>>>>> Add bindings for SolidRun Clearfog boards, using a new SoM based on
>>>>> CN9130 SoC.
>>>>> The carrier boards are identical to the older Armada 388 based Clearfog
>>>>> boards. For consistency the carrier part of compatible strings are
>>>>> copied, including the established "-a1" suffix.
>>>>>
>>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>>>> ---
>>>>>  .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml        | 12 ++++++++++++
>>>>>  1 file changed, 12 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>>>> index 16d2e132d3d1..36bdfd1bedd9 100644
>>>>> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>>>> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>>>> @@ -82,4 +82,16 @@ properties:
>>>>>            - const: marvell,armada-ap807-quad
>>>>>            - const: marvell,armada-ap807
>>>>>  
>>>>> +      - description:
>>>>> +          SolidRun CN9130 clearfog family single-board computers
>>>>> +        items:
>>>>> +          - enum:
>>>>> +              - solidrun,clearfog-base-a1
>>>>> +              - solidrun,clearfog-pro-a1
>>>>> +          - const: solidrun,clearfog-a1
>>>>> +          - const: solidrun,cn9130-sr-som
>>>>> +          - const: marvell,cn9130
>>>>> +          - const: marvell,armada-ap807-quad
>>>>> +          - const: marvell,armada-ap807
>>>>> +
>>>>>  additionalProperties: true
>>>> Before merging I would like some feedback about adding
>>>> another product later, to ensure the compatibles above
>>>> are adequate? In particular:
>>>> - sequence of soc, cp, carrier compatibles
>>>> - name of som compatible
>>>>
>>>> Draft for future bindings:
>>>>       - description:
>>>>           SolidRun CN9130 SoM based single-board computers
>>>>           with 1 external CP on the Carrier.
>>>>         items:
>>>>           - enum:
>>>>               - solidrun,cn9131-solidwan
>>>>           - const: marvell,cn9131
>>>>           - const: solidrun,cn9130-sr-som
>>> This does not look correct. cn9131 is not compatible with your som.
>> This is partially my question.
>> I considered changing the som to "cn913x-sr-som".
>>
>> The SoM itself is always 9130, it contains the base SoC
>> with 1x AP and 1x CP in a single chip.
>> 9131 and 9132 <happen> on the carrier boards.
> No wildcards, but if the SoM name is 9130 then use 9130.
> The problem is that you use cn9130 SoC as fallback.
>
>>>>           - const: marvell,cn9130
>>> SoCs are compatible only in some cases, e.g. one is a subset of another
>>> like stripped out of modem. Are you sure this is your case?
>> This is more complex, CN9131 and CN9132 are not single SoCs.
>> A "9132" is instantiated by connecting two southbridge chips
>> via a Marvell defined bus, each providing additional IO
>> such as network, i2c, gpio.
>>
>> Note that even the first, "9130", while a single chip, contains two dies:
>> An "AP" (Application Processor I assume) with very limited IO (1xsdio, 1xi2c),
>> and a "CP" (Communication Processor I assume) with lots of IO.
>> This CP as far as I know today is identical to the southbridges
>> mentioned above.
> OK, but how does it affect compatibility between them? Which parts are
> the same? Or how much is shared?
9130, 9131, 9132 belong together.
9130 is single chip including two dies: AP, CP.
The CP is available as an individual chip,
up to two can be connected to one 9130.

What does this mean for compatibility?
Which compatibility specifically?
Is there a definition we can refer to?

From software perspective we can always down-grade,
i.e. run software only aware of the AP on 9130, 9131 or 9132.
But we can't run software referencing the external CPs
if they are not connected.

>>>>           - const: marvell,armada-ap807-quad
>>>>           - const: marvell,armada-ap807
>>> Anyway, 6 compatibles is beyond useful amount. What are you expressing
>>> here?
>> I copied this part from the examples earlier in the file, such as:
>>       - description: Armada CN9132 SoC with two external CPs
>>         items:
>>           - const: marvell,cn9132
>>           - const: marvell,cn9131
>>           - const: marvell,cn9130
>>           - const: marvell,armada-ap807-quad
>>           - const: marvell,armada-ap807
>>>  Why is this even armada ap807?
>> We noticed ap807 != ap806 (cn913x != 8040),
>> because the thermal sensor coefficients converting
>> raw values to celsius differed.
> That's also not the best example.Might be correct but also looks
> over-complicated. The point of board-level compatibles is to identify
> machine and its common parts. It has little impact inside of kernel (at
> least should be almost no users inside!)
Indeed, the temperature coefficients are handled by the thermal device
compatible string, not board-level.
> , but there can be some users,
> e.g. firmware or user-space.
>
> This claims that cn9132 is compatible with ap807, so you have exactly
> the same base. The same base is not CPU! It's about the S in SoC, so
> "System".
I would think since the base is always a single chip combining 1x AP+CP,
the "system" is marvell,cn9130.
For Armada 8040, the system would be marvell,armada8040 by same
logic (also combining 1x AP+CP, different version, not extensible).
> Could firmware use marvell,armada-ap807 compatible to properly
> detect type of system and treat all these boards as ap807?
I have not looked into presence detection for CP's during initialization.
U-Boot support without spaghetti is a future Me task.
I suspect it is possible with asterisk *, because so far I have only seen
configuration with at least 1 CP, never with 0.
Presence of a boot-rom on each die e.g. supports this idea.


sincerely

Josua Mayer
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Andy Shevchenko @ 2024-03-26 19:21 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <ZgMf5eISwE2P_1tN@smile.fi.intel.com>

On Tue, Mar 26, 2024 at 09:20:05PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 26, 2024 at 07:10:09PM +0000, Mark Brown wrote:
> > On Tue, Mar 26, 2024 at 08:52:53PM +0200, Andy Shevchenko wrote:
> > > On Tue, Mar 26, 2024 at 06:49:58PM +0000, Mark Brown wrote:
> > 
> > > > > > > > I think the ACPI dependency there is as much about hiding the device on
> > > > > > > > irrelevant platforms as anything else, might be better replaced with an
> > > > > > > > x86 dependency though.
> > 
> > > Oh, oh, my bad I missed acpi_dev_uid_to_integer() call.
> > > Okay, with that in mind it's functional dependency for the ACPI-based
> > > platforms. Do you want to keep it untouched?
> > 
> > That's not actually what I was thinking of (please read what I wrote
> > above, like I say I was thining about hiding things) but surely if that
> > was a reason to keep the dependency it'd need to be an actual ACPI
> > dependency rather than an ||?
> 
> For my knowledge there is none of the ACPI-based platform where CONFIG_ACPI
> needs to be 'n' while having the real device (as per ACPI ID table) to be on.

to be on --> in a sense of "to be present".

> That's why I answered purely from the compilation point of view.
> 
> Personally I see that dependency more confusing than hinting about anything.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 0/4] dt-bindings: rtc: convert multiple devices to dtschema
From: Rob Herring @ 2024-03-26 19:20 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: Alexandre Belloni, Krzysztof Kozlowski, Conor Dooley,
	Baruch Siach, linux-rtc, devicetree, linux-kernel,
	linux-arm-kernel, Krzysztof Kozlowski
In-Reply-To: <20240326-rtc-yaml-v3-0-caa430ecace7@gmail.com>

On Tue, Mar 26, 2024 at 02:03:22PM +0100, Javier Carrasco wrote:
> This series converts the following existing bindings to dtschema:
> 
> - armada-380-rtc
> - alphascale,asm9260-rtc
> - digicolor-rtc (moved to trivial-rtc)
> - nxp,lpc1788-rtc
> 
> All bindings include at least one compatible that is referenced in the
> existing dts (arch/arm). Those dts could be tested against the new
> bindings.
> 
> It might be worth mentioning that the reference to nxp,lpc1788-rtc in
> arch/arm/boot/dts/nxp/lpc/lpc18xx.dtsi also includes another compatible
> called nxp,lpc1850-rtc, which is not documented or supported by existing
> drivers. That generates a warning when testing against nxp,lpc1788-rtc.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> Changes in v3:
> - alphascale-asm9260-rtc: drop unnecessary reg description.
> - nxp,lpc1788-rtc: drop unnecessary reg description.
> - Link to v2: https://lore.kernel.org/r/20240325-rtc-yaml-v2-0-ff9f68f43dbc@gmail.com
> 
> Changes in v2:
> - General: reference to rtc.yaml
> - digicolor-rtc: move to trivial-rtc
> - Link to v1: https://lore.kernel.org/r/20240323-rtc-yaml-v1-0-0c5d12b1b89d@gmail.com
> 
> ---
> Javier Carrasco (4):
>       dt-bindings: rtc: armada-380-rtc: convert to dtschema
>       dt-bindings: rtc: alphascale,asm9260-rtc: convert to dtschema
>       dt-bindings: rtc: digicolor-rtc: move to trivial-rtc
>       dt-bindings: rtc: nxp,lpc1788-rtc: convert to dtschema

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Andy Shevchenko @ 2024-03-26 19:20 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <4241ecb8-07e4-4613-a289-4699c39d0d08@sirena.org.uk>

On Tue, Mar 26, 2024 at 07:10:09PM +0000, Mark Brown wrote:
> On Tue, Mar 26, 2024 at 08:52:53PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 26, 2024 at 06:49:58PM +0000, Mark Brown wrote:
> 
> > > > > > > I think the ACPI dependency there is as much about hiding the device on
> > > > > > > irrelevant platforms as anything else, might be better replaced with an
> > > > > > > x86 dependency though.
> 
> > Oh, oh, my bad I missed acpi_dev_uid_to_integer() call.
> > Okay, with that in mind it's functional dependency for the ACPI-based
> > platforms. Do you want to keep it untouched?
> 
> That's not actually what I was thinking of (please read what I wrote
> above, like I say I was thining about hiding things) but surely if that
> was a reason to keep the dependency it'd need to be an actual ACPI
> dependency rather than an ||?

For my knowledge there is none of the ACPI-based platform where CONFIG_ACPI
needs to be 'n' while having the real device (as per ACPI ID table) to be on.
That's why I answered purely from the compilation point of view.

Personally I see that dependency more confusing than hinting about anything.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 1/4] remoteproc: Add TEE support
From: Arnaud POULIQUEN @ 2024-03-26 19:18 UTC (permalink / raw)
  To: Mathieu Poirier
  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: <ZgGqfItYtkUUvlxf@p14s>

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).

So please just comfirm your preference.

> 
>> +
>> +	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.

> 
>> +
>> +	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]
and we need to reapply the configuration in rproc_start() called during the
recovery[2]
[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

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
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 05/27] iommu/arm-smmu-v3: Make CD programming use arm_smmu_write_entry()
From: Mostafa Saleh @ 2024-03-26 19:12 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <20240326183055.GL6245@nvidia.com>

On Tue, Mar 26, 2024 at 03:30:55PM -0300, Jason Gunthorpe wrote:
> On Sat, Mar 23, 2024 at 01:02:15PM +0000, Mostafa Saleh wrote:
> > > +static void arm_smmu_get_cd_used(const __le64 *ent, __le64 *used_bits)
> > > +{
> > > +	used_bits[0] = cpu_to_le64(CTXDESC_CD_0_V);
> > > +	if (!(ent[0] & cpu_to_le64(CTXDESC_CD_0_V)))
> > > +		return;
> > > +	memset(used_bits, 0xFF, sizeof(struct arm_smmu_cd));
> > 
> > This is a slightly different approach than what the driver does for STEs,
> > where it explicitly sets the used bits. Is there a reason for that?
> 
> It is just more compact this way

IMHO, it seems too much to have this mechanism for CDs for just one
SVA case, but I'll need to go through the whole seires first to make
sure I am not missing anything.

> > > +	/* EPD0 means T0SZ/TG0/IR0/OR0/SH0/TTB0 are IGNORED */
> > > +	if (ent[0] & cpu_to_le64(CTXDESC_CD_0_TCR_EPD0)) {
> > > +		used_bits[0] &= ~cpu_to_le64(
> > > +			CTXDESC_CD_0_TCR_T0SZ | CTXDESC_CD_0_TCR_TG0 |
> > > +			CTXDESC_CD_0_TCR_IRGN0 | CTXDESC_CD_0_TCR_ORGN0 |
> > > +			CTXDESC_CD_0_TCR_SH0);
> > > +		used_bits[1] &= ~cpu_to_le64(CTXDESC_CD_1_TTB0_MASK);
> > > +	}
> > > +}
> > 
> > We should add a comment about EPD1 maybe?
> 
> Driver doesn't use TTB1?

Yes, it's not immediately obvious why we ignore EPD1, so maybe it's
worth a comment to highlight that, but no strong opinion.

Thanks,
Mostafa


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 0/4] arm64: dts: rockchip: add wolfvision pf5 mainboard
From: Rob Herring @ 2024-03-26 19:11 UTC (permalink / raw)
  To: Michael Riesch
  Cc: linux-kernel, linux-arm-kernel, linux-rockchip,
	Krzysztof Kozlowski, devicetree, Heiko Stuebner, Conor Dooley
In-Reply-To: <20240325-feature-wolfvision-pf5-v1-0-5725445f792a@wolfvision.net>


On Mon, 25 Mar 2024 15:22:30 +0100, Michael Riesch wrote:
> Habidere,
> 
> This series adds the device tree for the WolfVision PF5 mainboard, which
> serves as base for recent WolfVision products. It features the Rockchip
> RK3568 and can be extended with several different extension boards.
> 
> The WolfVision PF5 IO Expander is one example of such an extension board.
> The corresponding device tree overlay is also included in this series.
> 
> May this be the beginning of a beautiful friendship :-)
> 
> Looking forward to your comments!
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
> Michael Riesch (4):
>       dt-bindings: add wolfvision vendor prefix
>       dt-bindings: arm: rockchip: add wolfvision pf5 mainboard
>       arm64: dts: rockchip: add wolfvision pf5 mainboard
>       arm64: dts: rockchip: add wolfvision pf5 io expander board
> 
>  .../devicetree/bindings/arm/rockchip.yaml          |   5 +
>  .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
>  arch/arm64/boot/dts/rockchip/Makefile              |   2 +
>  .../rk3568-wolfvision-pf5-io-expander.dtso         | 137 ++++++
>  .../boot/dts/rockchip/rk3568-wolfvision-pf5.dts    | 528 +++++++++++++++++++++
>  5 files changed, 674 insertions(+)
> ---
> base-commit: 4cece764965020c22cff7665b18a012006359095
> change-id: 20240325-feature-wolfvision-pf5-5c1924c0389c
> 
> Best regards,
> --
> Michael Riesch <michael.riesch@wolfvision.net>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y rockchip/rk3568-wolfvision-pf5.dtb' for 20240325-feature-wolfvision-pf5-v1-0-5725445f792a@wolfvision.net:

arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5.dtb: hdmi@fe0a0000: Unevaluated properties are not allowed ('#sound-dai-cells' was unexpected)
	from schema $id: http://devicetree.org/schemas/display/rockchip/rockchip,dw-hdmi.yaml#






_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: rockchip: quartzpro64: Enable the GPU
From: Rob Herring @ 2024-03-26 19:11 UTC (permalink / raw)
  To: Dragan Simic
  Cc: conor+dt, kernel, devicetree, sebastian.reichel, heiko,
	linux-arm-kernel, boris.brezillon, linux-kernel, linux-rockchip,
	robh+dt, krzysztof.kozlowski+dt
In-Reply-To: <0f3759ee390f245dac447bbee038445ddfecbec0.1711383286.git.dsimic@manjaro.org>


On Mon, 25 Mar 2024 17:19:04 +0100, Dragan Simic wrote:
> Following the approach used to enable the Mali GPU on the rk3588-evb1, [1]
> do the same for the Pine64 QuartzPro64, which uses nearly identical hardware
> design as the RK3588 EVB1.
> 
> The slight disadvantage is that the regulator coupling logic requires the
> regulators to be always on, which is also noted in the comments.  This is
> obviously something to be improved at some point in the future, but should
> be fine for now, especially because the QuartzPro64 isn't a battery-powered
> board, so low power consumption isn't paramount.
> 
> [1] https://lore.kernel.org/linux-rockchip/20240325153850.189128-5-sebastian.reichel@collabora.com/
> 
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y rockchip/rk3588-quartzpro64.dtb' for 0f3759ee390f245dac447bbee038445ddfecbec0.1711383286.git.dsimic@manjaro.org:

Error: arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts:288.1-5 Label or path gpu not found
FATAL ERROR: Syntax error parsing input tree
make[3]: *** [scripts/Makefile.lib:427: arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dtb] Error 1
make[2]: *** [scripts/Makefile.build:485: arch/arm64/boot/dts/rockchip] Error 2
make[2]: Target 'arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dtb' not remade because of errors.
make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1387: rockchip/rk3588-quartzpro64.dtb] Error 2
make: *** [Makefile:240: __sub-make] Error 2
make: Target 'rockchip/rk3588-quartzpro64.dtb' not remade because of errors.






_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Mark Brown @ 2024-03-26 19:10 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <ZgMZhdsDc-bzWa6P@smile.fi.intel.com>


[-- Attachment #1.1: Type: text/plain, Size: 748 bytes --]

On Tue, Mar 26, 2024 at 08:52:53PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 26, 2024 at 06:49:58PM +0000, Mark Brown wrote:

> > > > > > I think the ACPI dependency there is as much about hiding the device on
> > > > > > irrelevant platforms as anything else, might be better replaced with an
> > > > > > x86 dependency though.

> Oh, oh, my bad I missed acpi_dev_uid_to_integer() call.
> Okay, with that in mind it's functional dependency for the ACPI-based
> platforms. Do you want to keep it untouched?

That's not actually what I was thinking of (please read what I wrote
above, like I say I was thining about hiding things) but surely if that
was a reason to keep the dependency it'd need to be an actual ACPI
dependency rather than an ||?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 01/27] iommu/arm-smmu-v3: Do not allow a SVA domain to be set on the wrong PASID
From: Mostafa Saleh @ 2024-03-26 19:06 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <20240326183016.GK6245@nvidia.com>

On Tue, Mar 26, 2024 at 03:30:16PM -0300, Jason Gunthorpe wrote:
> On Fri, Mar 22, 2024 at 05:48:52PM +0000, Mostafa Saleh wrote:
> > Hi Jason,
> > 
> > On Mon, Mar 04, 2024 at 07:43:49PM -0400, Jason Gunthorpe wrote:
> > > The SVA code is wired to assume that the SVA is programmed onto the
> > > mm->pasid. The current core code always does this, so it is fine.
> > > 
> > > Add a check for clarity.
> > > 
> > > Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > > ---
> > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > index 2610e82c0ecd0d..347c2fdd865c1a 100644
> > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > @@ -581,6 +581,9 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
> > >  	int ret = 0;
> > >  	struct mm_struct *mm = domain->mm;
> > >  
> > > +	if (mm_get_enqcmd_pasid(mm) != id)
> > > +		return -EINVAL;
> > > +
> > I am not sure if that is needed, the only caller in the tree is the IOMMU code
> > and it does the right thing, as that check is removed later anyway, I don’t
> > think this patch adds much.
> 
> It really should be backported, when we get drivers that do other
> things it creates a hazard. I've added a fixes line.

Maybe I am misunderstanding the case, but AFAIU, the only caller for this is
iommu_sva_bind_device() which is the function populating the pasid, so this
condition should never hit with the current code.
And Linux won’t backport new drivers, so there is no need to do that?

Thanks,
Mostafa

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


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