[PATCH 0/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Drive-by fixes

[PATCH 0/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Drive-by fixes

[Laurent Pinchart]

Hello,

This small series includes two drive-by fixes to the
raspberrypi,bcm2835-firmware DT bindings that fix validation errors with
the Raspberry Pi 4 device tree sources. I noticed those issues when
working on the Raspberry Pi Unicam driver, but two patches are
independent of that work, they can thus be applied separately.

Laurent Pinchart (2):
  dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing
    properties
  dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child
    node

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

-- 
Regards,

Laurent Pinchart

[PATCH 0/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Drive-by fixes

[Laurent Pinchart]

Hello,

This small series includes two drive-by fixes to the
raspberrypi,bcm2835-firmware DT bindings that fix validation errors with
the Raspberry Pi 4 device tree sources. I noticed those issues when
working on the Raspberry Pi Unicam driver, but two patches are
independent of that work, they can thus be applied separately.

Laurent Pinchart (2):
  dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing
    properties
  dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child
    node

 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 45 +++++++++++++++++++
 .../gpio/raspberrypi,firmware-gpio.txt        | 30 -------------
 2 files changed, 45 insertions(+), 30 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

[PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Laurent Pinchart]

The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
and, as a result, also needs to specify #address-cells and #size-cells.
Those properties have been added to thebcm2835-rpi.dtsi in commits
be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
limitations"), but the DT bindings haven't been updated, resulting in
validation errors:

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

Fix this by adding the properties to the bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml    | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index 39e3c248f5b7..dc38f2be7ad6 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -25,6 +25,14 @@ properties:
       - const: raspberrypi,bcm2835-firmware
       - const: simple-mfd
 
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  dma-ranges: true
+
   mboxes:
     maxItems: 1
 
@@ -81,6 +89,9 @@ properties:
 
 required:
   - compatible
+  - "#address-cells"
+  - "#size-cells"
+  - dma-ranges
   - mboxes
 
 additionalProperties: false
@@ -89,6 +100,11 @@ examples:
   - |
     firmware {
         compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+        dma-ranges;
+
         mboxes = <&mailbox>;
 
         firmware_clocks: clocks {
-- 
Regards,

Laurent Pinchart

[PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Laurent Pinchart]

The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
and, as a result, also needs to specify #address-cells and #size-cells.
Those properties have been added to thebcm2835-rpi.dtsi in commits
be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
limitations"), but the DT bindings haven't been updated, resulting in
validation errors:

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

Fix this by adding the properties to the bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml    | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index 39e3c248f5b7..dc38f2be7ad6 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -25,6 +25,14 @@ properties:
       - const: raspberrypi,bcm2835-firmware
       - const: simple-mfd
 
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  dma-ranges: true
+
   mboxes:
     maxItems: 1
 
@@ -81,6 +89,9 @@ properties:
 
 required:
   - compatible
+  - "#address-cells"
+  - "#size-cells"
+  - dma-ranges
   - mboxes
 
 additionalProperties: false
@@ -89,6 +100,11 @@ examples:
   - |
     firmware {
         compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+        dma-ranges;
+
         mboxes = <&mailbox>;
 
         firmware_clocks: clocks {
-- 
Regards,

Laurent Pinchart


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

[PATCH 2/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node

[Laurent Pinchart]

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>
---
 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 29 ++++++++++++++++++
 .../gpio/raspberrypi,firmware-gpio.txt        | 30 -------------------
 2 files changed, 29 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 dc38f2be7ad6..999e1bc49539 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -54,6 +54,29 @@ 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: true
+
+    required:
+      - compatible
+      - gpio-controller
+      - "#gpio-cells"
+
   reset:
     type: object
     additionalProperties: false
@@ -112,6 +135,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

[PATCH 2/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node

[Laurent Pinchart]

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>
---
 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 29 ++++++++++++++++++
 .../gpio/raspberrypi,firmware-gpio.txt        | 30 -------------------
 2 files changed, 29 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 dc38f2be7ad6..999e1bc49539 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -54,6 +54,29 @@ 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: true
+
+    required:
+      - compatible
+      - gpio-controller
+      - "#gpio-cells"
+
   reset:
     type: object
     additionalProperties: false
@@ -112,6 +135,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

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Krzysztof Kozlowski]

On 26/03/2024 01:49, Laurent Pinchart wrote:
> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> and, as a result, also needs to specify #address-cells and #size-cells.
> Those properties have been added to thebcm2835-rpi.dtsi in commits
> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> limitations"), but the DT bindings haven't been updated, resulting in
> validation errors:
> 
> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
>         from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> 
> Fix this by adding the properties to the bindings.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Children do not perform any IO on their own, because everything is
handled by parent. It is really odd to see dma-ranges without ranges.
Referenced commits might be also wrong.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Krzysztof Kozlowski]

On 26/03/2024 01:49, Laurent Pinchart wrote:
> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> and, as a result, also needs to specify #address-cells and #size-cells.
> Those properties have been added to thebcm2835-rpi.dtsi in commits
> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> limitations"), but the DT bindings haven't been updated, resulting in
> validation errors:
> 
> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
>         from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> 
> Fix this by adding the properties to the bindings.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Children do not perform any IO on their own, because everything is
handled by parent. It is really odd to see dma-ranges without ranges.
Referenced commits might be also wrong.

Best regards,
Krzysztof


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

Re: [PATCH 2/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node

[Krzysztof Kozlowski]

On 26/03/2024 01:49, Laurent Pinchart wrote:
> 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:



> +    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: true

You could provide here maxItems, if this is known, but it's fine as is
as well.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 2/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node

[Krzysztof Kozlowski]

On 26/03/2024 01:49, Laurent Pinchart wrote:
> 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:



> +    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: true

You could provide here maxItems, if this is known, but it's fine as is
as well.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


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

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Stefan Wahren]

[add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
opinion about this]

[drop Emma Anholt old address since she is not involved anymore]

Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> On 26/03/2024 01:49, Laurent Pinchart wrote:
>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
>> and, as a result, also needs to specify #address-cells and #size-cells.
>> Those properties have been added to thebcm2835-rpi.dtsi in commits
>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
>> limitations"), but the DT bindings haven't been updated, resulting in
>> validation errors:
>>
>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
>>          from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
>>
>> Fix this by adding the properties to the bindings.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Children do not perform any IO on their own, because everything is
> handled by parent. It is really odd to see dma-ranges without ranges.
> Referenced commits might be also wrong.
>
> Best regards,
> Krzysztof
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Stefan Wahren]

[add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
opinion about this]

[drop Emma Anholt old address since she is not involved anymore]

Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> On 26/03/2024 01:49, Laurent Pinchart wrote:
>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
>> and, as a result, also needs to specify #address-cells and #size-cells.
>> Those properties have been added to thebcm2835-rpi.dtsi in commits
>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
>> limitations"), but the DT bindings haven't been updated, resulting in
>> validation errors:
>>
>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
>>          from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
>>
>> Fix this by adding the properties to the bindings.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Children do not perform any IO on their own, because everything is
> handled by parent. It is really odd to see dma-ranges without ranges.
> Referenced commits might be also wrong.
>
> Best regards,
> Krzysztof
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


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

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Laurent Pinchart]

On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> opinion about this]
> 
> [drop Emma Anholt old address since she is not involved anymore]
> 
> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> > On 26/03/2024 01:49, Laurent Pinchart wrote:
> >> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >> and, as a result, also needs to specify #address-cells and #size-cells.
> >> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >> limitations"), but the DT bindings haven't been updated, resulting in
> >> validation errors:
> >>
> >> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >>          from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>
> >> Fix this by adding the properties to the bindings.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Children do not perform any IO on their own, because everything is
> > handled by parent. It is really odd to see dma-ranges without ranges.
> > Referenced commits might be also wrong.

Comunication with the firmware goes through a mailbox interface, which
uses DMA transfers. See for instance

rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
{
	u32 message = MBOX_MSG(chan, data);
	int ret;

	WARN_ON(data & 0xf);

	mutex_lock(&transaction_lock);
	reinit_completion(&fw->c);
	ret = mbox_send_message(fw->chan, &message);
	if (ret >= 0) {
		if (wait_for_completion_timeout(&fw->c, HZ)) {
			ret = 0;
		} else {
			ret = -ETIMEDOUT;
			WARN_ONCE(1, "Firmware transaction timeout");
		}
	} else {
		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
	}
	mutex_unlock(&transaction_lock);

	return ret;
}

int rpi_firmware_property_list(struct rpi_firmware *fw,
			       void *data, size_t tag_size)
{
	size_t size = tag_size + 12;
	u32 *buf;
	dma_addr_t bus_addr;
	int ret;

	/* Packets are processed a dword at a time. */
	if (size & 3)
		return -EINVAL;

	buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
				 GFP_ATOMIC);
	if (!buf)
		return -ENOMEM;

	/* The firmware will error out without parsing in this case. */
	WARN_ON(size >= 1024 * 1024);

	buf[0] = size;
	buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
	memcpy(&buf[2], data, tag_size);
	buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
	wmb();

	ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);

	rmb();
	memcpy(data, &buf[2], tag_size);
	if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
		/*
		 * The tag name here might not be the one causing the
		 * error, if there were multiple tags in the request.
		 * But single-tag is the most common, so go with it.
		 */
		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
			buf[2], buf[1]);
		ret = -EINVAL;
	}

	dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);

	return ret;
}

fw->cl.dev is the device for the firmware child node. That may be where
the problem comes from, shouldn't we use the mailbox device for DMA
mapping ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Laurent Pinchart]

On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> opinion about this]
> 
> [drop Emma Anholt old address since she is not involved anymore]
> 
> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> > On 26/03/2024 01:49, Laurent Pinchart wrote:
> >> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >> and, as a result, also needs to specify #address-cells and #size-cells.
> >> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >> limitations"), but the DT bindings haven't been updated, resulting in
> >> validation errors:
> >>
> >> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >>          from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>
> >> Fix this by adding the properties to the bindings.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Children do not perform any IO on their own, because everything is
> > handled by parent. It is really odd to see dma-ranges without ranges.
> > Referenced commits might be also wrong.

Comunication with the firmware goes through a mailbox interface, which
uses DMA transfers. See for instance

rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
{
	u32 message = MBOX_MSG(chan, data);
	int ret;

	WARN_ON(data & 0xf);

	mutex_lock(&transaction_lock);
	reinit_completion(&fw->c);
	ret = mbox_send_message(fw->chan, &message);
	if (ret >= 0) {
		if (wait_for_completion_timeout(&fw->c, HZ)) {
			ret = 0;
		} else {
			ret = -ETIMEDOUT;
			WARN_ONCE(1, "Firmware transaction timeout");
		}
	} else {
		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
	}
	mutex_unlock(&transaction_lock);

	return ret;
}

int rpi_firmware_property_list(struct rpi_firmware *fw,
			       void *data, size_t tag_size)
{
	size_t size = tag_size + 12;
	u32 *buf;
	dma_addr_t bus_addr;
	int ret;

	/* Packets are processed a dword at a time. */
	if (size & 3)
		return -EINVAL;

	buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
				 GFP_ATOMIC);
	if (!buf)
		return -ENOMEM;

	/* The firmware will error out without parsing in this case. */
	WARN_ON(size >= 1024 * 1024);

	buf[0] = size;
	buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
	memcpy(&buf[2], data, tag_size);
	buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
	wmb();

	ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);

	rmb();
	memcpy(data, &buf[2], tag_size);
	if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
		/*
		 * The tag name here might not be the one causing the
		 * error, if there were multiple tags in the request.
		 * But single-tag is the most common, so go with it.
		 */
		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
			buf[2], buf[1]);
		ret = -EINVAL;
	}

	dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);

	return ret;
}

fw->cl.dev is the device for the firmware child node. That may be where
the problem comes from, shouldn't we use the mailbox device for DMA
mapping ?

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 2/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node

[Laurent Pinchart]

On Tue, Mar 26, 2024 at 08:09:46AM +0100, Krzysztof Kozlowski wrote:
> On 26/03/2024 01:49, Laurent Pinchart wrote:
> > 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:
> 
> > +    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: true
> 
> You could provide here maxItems, if this is known, but it's fine as is
> as well.

The number of items seems to be always 8, so I'll use that value.

> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node

[Laurent Pinchart]

On Tue, Mar 26, 2024 at 08:09:46AM +0100, Krzysztof Kozlowski wrote:
> On 26/03/2024 01:49, Laurent Pinchart wrote:
> > 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:
> 
> > +    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: true
> 
> You could provide here maxItems, if this is known, but it's fine as is
> as well.

The number of items seems to be always 8, so I'll use that value.

> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Stefan Wahren]

Am 26.03.24 um 18:18 schrieb Laurent Pinchart:
> On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
>> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
>> opinion about this]
>>
>> [drop Emma Anholt old address since she is not involved anymore]
>>
>> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
>>> On 26/03/2024 01:49, Laurent Pinchart wrote:
>>>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
>>>> and, as a result, also needs to specify #address-cells and #size-cells.
>>>> Those properties have been added to thebcm2835-rpi.dtsi in commits
>>>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
>>>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
>>>> limitations"), but the DT bindings haven't been updated, resulting in
>>>> validation errors:
>>>>
>>>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
>>>>           from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
>>>>
>>>> Fix this by adding the properties to the bindings.
>>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Children do not perform any IO on their own, because everything is
>>> handled by parent. It is really odd to see dma-ranges without ranges.
>>> Referenced commits might be also wrong.
> Comunication with the firmware goes through a mailbox interface, which
> uses DMA transfers. See for instance
>
> rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
> {
> 	u32 message = MBOX_MSG(chan, data);
> 	int ret;
>
> 	WARN_ON(data & 0xf);
>
> 	mutex_lock(&transaction_lock);
> 	reinit_completion(&fw->c);
> 	ret = mbox_send_message(fw->chan, &message);
> 	if (ret >= 0) {
> 		if (wait_for_completion_timeout(&fw->c, HZ)) {
> 			ret = 0;
> 		} else {
> 			ret = -ETIMEDOUT;
> 			WARN_ONCE(1, "Firmware transaction timeout");
> 		}
> 	} else {
> 		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
> 	}
> 	mutex_unlock(&transaction_lock);
>
> 	return ret;
> }
>
> int rpi_firmware_property_list(struct rpi_firmware *fw,
> 			       void *data, size_t tag_size)
> {
> 	size_t size = tag_size + 12;
> 	u32 *buf;
> 	dma_addr_t bus_addr;
> 	int ret;
>
> 	/* Packets are processed a dword at a time. */
> 	if (size & 3)
> 		return -EINVAL;
>
> 	buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
> 				 GFP_ATOMIC);
> 	if (!buf)
> 		return -ENOMEM;
>
> 	/* The firmware will error out without parsing in this case. */
> 	WARN_ON(size >= 1024 * 1024);
>
> 	buf[0] = size;
> 	buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
> 	memcpy(&buf[2], data, tag_size);
> 	buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
> 	wmb();
>
> 	ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
>
> 	rmb();
> 	memcpy(data, &buf[2], tag_size);
> 	if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
> 		/*
> 		 * The tag name here might not be the one causing the
> 		 * error, if there were multiple tags in the request.
> 		 * But single-tag is the most common, so go with it.
> 		 */
> 		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
> 			buf[2], buf[1]);
> 		ret = -EINVAL;
> 	}
>
> 	dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
>
> 	return ret;
> }
>
> fw->cl.dev is the device for the firmware child node. That may be where
> the problem comes from, shouldn't we use the mailbox device for DMA
> mapping ?
>

 From devicetree perspective this is the mailbox DT part [1] and this
the matching dt-binding [2].

[1] -
https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm/boot/dts/broadcom/bcm283x.dtsi#L100
[2] -
https://elixir.bootlin.com/linux/v6.9-rc1/source/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.yaml

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Stefan Wahren]

Am 26.03.24 um 18:18 schrieb Laurent Pinchart:
> On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
>> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
>> opinion about this]
>>
>> [drop Emma Anholt old address since she is not involved anymore]
>>
>> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
>>> On 26/03/2024 01:49, Laurent Pinchart wrote:
>>>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
>>>> and, as a result, also needs to specify #address-cells and #size-cells.
>>>> Those properties have been added to thebcm2835-rpi.dtsi in commits
>>>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
>>>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
>>>> limitations"), but the DT bindings haven't been updated, resulting in
>>>> validation errors:
>>>>
>>>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
>>>>           from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
>>>>
>>>> Fix this by adding the properties to the bindings.
>>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Children do not perform any IO on their own, because everything is
>>> handled by parent. It is really odd to see dma-ranges without ranges.
>>> Referenced commits might be also wrong.
> Comunication with the firmware goes through a mailbox interface, which
> uses DMA transfers. See for instance
>
> rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
> {
> 	u32 message = MBOX_MSG(chan, data);
> 	int ret;
>
> 	WARN_ON(data & 0xf);
>
> 	mutex_lock(&transaction_lock);
> 	reinit_completion(&fw->c);
> 	ret = mbox_send_message(fw->chan, &message);
> 	if (ret >= 0) {
> 		if (wait_for_completion_timeout(&fw->c, HZ)) {
> 			ret = 0;
> 		} else {
> 			ret = -ETIMEDOUT;
> 			WARN_ONCE(1, "Firmware transaction timeout");
> 		}
> 	} else {
> 		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
> 	}
> 	mutex_unlock(&transaction_lock);
>
> 	return ret;
> }
>
> int rpi_firmware_property_list(struct rpi_firmware *fw,
> 			       void *data, size_t tag_size)
> {
> 	size_t size = tag_size + 12;
> 	u32 *buf;
> 	dma_addr_t bus_addr;
> 	int ret;
>
> 	/* Packets are processed a dword at a time. */
> 	if (size & 3)
> 		return -EINVAL;
>
> 	buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
> 				 GFP_ATOMIC);
> 	if (!buf)
> 		return -ENOMEM;
>
> 	/* The firmware will error out without parsing in this case. */
> 	WARN_ON(size >= 1024 * 1024);
>
> 	buf[0] = size;
> 	buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
> 	memcpy(&buf[2], data, tag_size);
> 	buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
> 	wmb();
>
> 	ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
>
> 	rmb();
> 	memcpy(data, &buf[2], tag_size);
> 	if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
> 		/*
> 		 * The tag name here might not be the one causing the
> 		 * error, if there were multiple tags in the request.
> 		 * But single-tag is the most common, so go with it.
> 		 */
> 		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
> 			buf[2], buf[1]);
> 		ret = -EINVAL;
> 	}
>
> 	dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
>
> 	return ret;
> }
>
> fw->cl.dev is the device for the firmware child node. That may be where
> the problem comes from, shouldn't we use the mailbox device for DMA
> mapping ?
>

 From devicetree perspective this is the mailbox DT part [1] and this
the matching dt-binding [2].

[1] -
https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm/boot/dts/broadcom/bcm283x.dtsi#L100
[2] -
https://elixir.bootlin.com/linux/v6.9-rc1/source/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.yaml

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

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Laurent Pinchart]

On Tue, Mar 26, 2024 at 06:40:52PM +0100, Stefan Wahren wrote:
> Am 26.03.24 um 18:18 schrieb Laurent Pinchart:
> > On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
> >> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> >> opinion about this]
> >>
> >> [drop Emma Anholt old address since she is not involved anymore]
> >>
> >> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> >>> On 26/03/2024 01:49, Laurent Pinchart wrote:
> >>>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >>>> and, as a result, also needs to specify #address-cells and #size-cells.
> >>>> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >>>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >>>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >>>> limitations"), but the DT bindings haven't been updated, resulting in
> >>>> validation errors:
> >>>>
> >>>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >>>>           from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>>>
> >>>> Fix this by adding the properties to the bindings.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>> Children do not perform any IO on their own, because everything is
> >>> handled by parent. It is really odd to see dma-ranges without ranges.
> >>> Referenced commits might be also wrong.
> >
> > Comunication with the firmware goes through a mailbox interface, which
> > uses DMA transfers. See for instance
> >
> > rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
> > {
> > 	u32 message = MBOX_MSG(chan, data);
> > 	int ret;
> >
> > 	WARN_ON(data & 0xf);
> >
> > 	mutex_lock(&transaction_lock);
> > 	reinit_completion(&fw->c);
> > 	ret = mbox_send_message(fw->chan, &message);
> > 	if (ret >= 0) {
> > 		if (wait_for_completion_timeout(&fw->c, HZ)) {
> > 			ret = 0;
> > 		} else {
> > 			ret = -ETIMEDOUT;
> > 			WARN_ONCE(1, "Firmware transaction timeout");
> > 		}
> > 	} else {
> > 		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
> > 	}
> > 	mutex_unlock(&transaction_lock);
> >
> > 	return ret;
> > }
> >
> > int rpi_firmware_property_list(struct rpi_firmware *fw,
> > 			       void *data, size_t tag_size)
> > {
> > 	size_t size = tag_size + 12;
> > 	u32 *buf;
> > 	dma_addr_t bus_addr;
> > 	int ret;
> >
> > 	/* Packets are processed a dword at a time. */
> > 	if (size & 3)
> > 		return -EINVAL;
> >
> > 	buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
> > 				 GFP_ATOMIC);
> > 	if (!buf)
> > 		return -ENOMEM;
> >
> > 	/* The firmware will error out without parsing in this case. */
> > 	WARN_ON(size >= 1024 * 1024);
> >
> > 	buf[0] = size;
> > 	buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
> > 	memcpy(&buf[2], data, tag_size);
> > 	buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
> > 	wmb();
> >
> > 	ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
> >
> > 	rmb();
> > 	memcpy(data, &buf[2], tag_size);
> > 	if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
> > 		/*
> > 		 * The tag name here might not be the one causing the
> > 		 * error, if there were multiple tags in the request.
> > 		 * But single-tag is the most common, so go with it.
> > 		 */
> > 		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
> > 			buf[2], buf[1]);
> > 		ret = -EINVAL;
> > 	}
> >
> > 	dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
> >
> > 	return ret;
> > }
> >
> > fw->cl.dev is the device for the firmware child node. That may be where
> > the problem comes from, shouldn't we use the mailbox device for DMA
> > mapping ?
> 
>  From devicetree perspective this is the mailbox DT part [1] and this
> the matching dt-binding [2].
> 
> [1] - https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm/boot/dts/broadcom/bcm283x.dtsi#L100
> [2] - https://elixir.bootlin.com/linux/v6.9-rc1/source/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.yaml

That's the device performing DMA, so I think it should be used for DMA
mapping.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Laurent Pinchart]

On Tue, Mar 26, 2024 at 06:40:52PM +0100, Stefan Wahren wrote:
> Am 26.03.24 um 18:18 schrieb Laurent Pinchart:
> > On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
> >> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> >> opinion about this]
> >>
> >> [drop Emma Anholt old address since she is not involved anymore]
> >>
> >> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> >>> On 26/03/2024 01:49, Laurent Pinchart wrote:
> >>>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >>>> and, as a result, also needs to specify #address-cells and #size-cells.
> >>>> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >>>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >>>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >>>> limitations"), but the DT bindings haven't been updated, resulting in
> >>>> validation errors:
> >>>>
> >>>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >>>>           from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>>>
> >>>> Fix this by adding the properties to the bindings.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>> Children do not perform any IO on their own, because everything is
> >>> handled by parent. It is really odd to see dma-ranges without ranges.
> >>> Referenced commits might be also wrong.
> >
> > Comunication with the firmware goes through a mailbox interface, which
> > uses DMA transfers. See for instance
> >
> > rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
> > {
> > 	u32 message = MBOX_MSG(chan, data);
> > 	int ret;
> >
> > 	WARN_ON(data & 0xf);
> >
> > 	mutex_lock(&transaction_lock);
> > 	reinit_completion(&fw->c);
> > 	ret = mbox_send_message(fw->chan, &message);
> > 	if (ret >= 0) {
> > 		if (wait_for_completion_timeout(&fw->c, HZ)) {
> > 			ret = 0;
> > 		} else {
> > 			ret = -ETIMEDOUT;
> > 			WARN_ONCE(1, "Firmware transaction timeout");
> > 		}
> > 	} else {
> > 		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
> > 	}
> > 	mutex_unlock(&transaction_lock);
> >
> > 	return ret;
> > }
> >
> > int rpi_firmware_property_list(struct rpi_firmware *fw,
> > 			       void *data, size_t tag_size)
> > {
> > 	size_t size = tag_size + 12;
> > 	u32 *buf;
> > 	dma_addr_t bus_addr;
> > 	int ret;
> >
> > 	/* Packets are processed a dword at a time. */
> > 	if (size & 3)
> > 		return -EINVAL;
> >
> > 	buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
> > 				 GFP_ATOMIC);
> > 	if (!buf)
> > 		return -ENOMEM;
> >
> > 	/* The firmware will error out without parsing in this case. */
> > 	WARN_ON(size >= 1024 * 1024);
> >
> > 	buf[0] = size;
> > 	buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
> > 	memcpy(&buf[2], data, tag_size);
> > 	buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
> > 	wmb();
> >
> > 	ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
> >
> > 	rmb();
> > 	memcpy(data, &buf[2], tag_size);
> > 	if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
> > 		/*
> > 		 * The tag name here might not be the one causing the
> > 		 * error, if there were multiple tags in the request.
> > 		 * But single-tag is the most common, so go with it.
> > 		 */
> > 		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
> > 			buf[2], buf[1]);
> > 		ret = -EINVAL;
> > 	}
> >
> > 	dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
> >
> > 	return ret;
> > }
> >
> > fw->cl.dev is the device for the firmware child node. That may be where
> > the problem comes from, shouldn't we use the mailbox device for DMA
> > mapping ?
> 
>  From devicetree perspective this is the mailbox DT part [1] and this
> the matching dt-binding [2].
> 
> [1] - https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm/boot/dts/broadcom/bcm283x.dtsi#L100
> [2] - https://elixir.bootlin.com/linux/v6.9-rc1/source/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.yaml

That's the device performing DMA, so I think it should be used for DMA
mapping.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Dave Stevenson]

On Tue, 26 Mar 2024 at 11:47, Stefan Wahren <wahrenst@gmx.net> wrote:
>
> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> opinion about this]

No real opinion from me, but I am far from a DT expert, and AFAIK this
bit isn't impacted by the stuff I'm looking at.

Laurent is correct that it's missing from the binding doc when it
looks to be needed. Adding it would therefore be the correct thing to
do.

  Dave

> [drop Emma Anholt old address since she is not involved anymore]
>
> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> > On 26/03/2024 01:49, Laurent Pinchart wrote:
> >> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >> and, as a result, also needs to specify #address-cells and #size-cells.
> >> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >> limitations"), but the DT bindings haven't been updated, resulting in
> >> validation errors:
> >>
> >> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >>          from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>
> >> Fix this by adding the properties to the bindings.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Children do not perform any IO on their own, because everything is
> > handled by parent. It is really odd to see dma-ranges without ranges.
> > Referenced commits might be also wrong.
> >
> > Best regards,
> > Krzysztof
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Dave Stevenson]

On Tue, 26 Mar 2024 at 11:47, Stefan Wahren <wahrenst@gmx.net> wrote:
>
> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> opinion about this]

No real opinion from me, but I am far from a DT expert, and AFAIK this
bit isn't impacted by the stuff I'm looking at.

Laurent is correct that it's missing from the binding doc when it
looks to be needed. Adding it would therefore be the correct thing to
do.

  Dave

> [drop Emma Anholt old address since she is not involved anymore]
>
> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> > On 26/03/2024 01:49, Laurent Pinchart wrote:
> >> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >> and, as a result, also needs to specify #address-cells and #size-cells.
> >> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >> limitations"), but the DT bindings haven't been updated, resulting in
> >> validation errors:
> >>
> >> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >>          from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>
> >> Fix this by adding the properties to the bindings.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Children do not perform any IO on their own, because everything is
> > handled by parent. It is really odd to see dma-ranges without ranges.
> > Referenced commits might be also wrong.
> >
> > Best regards,
> > Krzysztof
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

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

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Rob Herring]

On Tue, Mar 26, 2024 at 02:49:01AM +0200, Laurent Pinchart wrote:
> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> and, as a result, also needs to specify #address-cells and #size-cells.
> Those properties have been added to thebcm2835-rpi.dtsi in commits
> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> limitations"), but the DT bindings haven't been updated, resulting in
> validation errors:

I don't understand. We treat no dma-ranges the same as empty dma-ranges 
(dma-ranges;). If we didn't, *every* DT would be broken.

We should never have dma-ranges without ranges either. 

Rob

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Rob Herring]

On Tue, Mar 26, 2024 at 02:49:01AM +0200, Laurent Pinchart wrote:
> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> and, as a result, also needs to specify #address-cells and #size-cells.
> Those properties have been added to thebcm2835-rpi.dtsi in commits
> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> limitations"), but the DT bindings haven't been updated, resulting in
> validation errors:

I don't understand. We treat no dma-ranges the same as empty dma-ranges 
(dma-ranges;). If we didn't, *every* DT would be broken.

We should never have dma-ranges without ranges either. 

Rob

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

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Laurent Pinchart]

Hi Rob,

On Tue, Mar 26, 2024 at 04:30:53PM -0500, Rob Herring wrote:
> On Tue, Mar 26, 2024 at 02:49:01AM +0200, Laurent Pinchart wrote:
> > The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> > and, as a result, also needs to specify #address-cells and #size-cells.
> > Those properties have been added to thebcm2835-rpi.dtsi in commits
> > be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> > bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> > limitations"), but the DT bindings haven't been updated, resulting in
> > validation errors:
> 
> I don't understand. We treat no dma-ranges the same as empty dma-ranges 
> (dma-ranges;). If we didn't, *every* DT would be broken.
> 
> We should never have dma-ranges without ranges either. 

Please see v2 :-)

https://lore.kernel.org/linux-arm-kernel/20240326195807.15163-3-laurent.pinchart@ideasonboard.com/
https://lore.kernel.org/linux-arm-kernel/20240326195807.15163-4-laurent.pinchart@ideasonboard.com/

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties

[Laurent Pinchart]

Hi Rob,

On Tue, Mar 26, 2024 at 04:30:53PM -0500, Rob Herring wrote:
> On Tue, Mar 26, 2024 at 02:49:01AM +0200, Laurent Pinchart wrote:
> > The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> > and, as a result, also needs to specify #address-cells and #size-cells.
> > Those properties have been added to thebcm2835-rpi.dtsi in commits
> > be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> > bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> > limitations"), but the DT bindings haven't been updated, resulting in
> > validation errors:
> 
> I don't understand. We treat no dma-ranges the same as empty dma-ranges 
> (dma-ranges;). If we didn't, *every* DT would be broken.
> 
> We should never have dma-ranges without ranges either. 

Please see v2 :-)

https://lore.kernel.org/linux-arm-kernel/20240326195807.15163-3-laurent.pinchart@ideasonboard.com/
https://lore.kernel.org/linux-arm-kernel/20240326195807.15163-4-laurent.pinchart@ideasonboard.com/

-- 
Regards,

Laurent Pinchart

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