* [PATCH v2 0/4] Improve VCHIQ cache line size handling
@ 2018-09-14 13:22 Phil Elwell
2018-09-14 13:22 ` [PATCH v2 1/4] staging/vc04_services: Use correct cache line size Phil Elwell
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Phil Elwell @ 2018-09-14 13:22 UTC (permalink / raw)
To: Rob Herring, Stefan Wahren, Greg Kroah-Hartman, Phil Elwell,
devicetree, linux-rpi-kernel, Russell King, Arnd Bergmann,
linux-arm-kernel, bcm-kernel-feedback-list, devel
Both sides of the VCHIQ communications mechanism need to agree on the cache
line size. Using an incorrect value can lead to data corruption, but having the
two sides using different values is usually worse.
In the absence of an obvious convenient run-time method to determine the
correct value in the ARCH=arm world, the downstream Raspberry Pi trees used a
Device Tree property, written by the firmware, to configure the kernel driver.
This method was vetoed during the upstreaming process, so a fixed value of 32
was used instead, and some corruptions ensued. This is take 2 at arriving at
the correct value.
Add a new compatible string - "brcm,bcm2836-vchiq" - to indicate an SoC with
a 64-byte cache line. Document the new string in the binding, and use it on
the appropriate platforms.
The final patch is a (seemingly cosmetic) correction of the Device Tree "reg"
declaration for the device node, but it doubles as an indication to the
Raspberry Pi firmware that the kernel driver is running a recent kernel driver
that chooses the correct value. As such it would help if the DT patches are
not merged before the driver patch.
v2: Replaced ARM-specific logic used to determine cache line size with
a new compatible string for BCM2836 and BCM2837.
Phil Elwell (4):
staging/vc04_services: Use correct cache line size
dt-bindings: soc: Document "brcm,bcm2836-vchiq"
ARM: dts: bcm283x: Correct vchiq compatible string
ARM: dts: bcm283x: Correct mailbox register sizes
.../bindings/soc/bcm/brcm,bcm2835-vchiq.txt | 3 +-
arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 +--
arch/arm/boot/dts/bcm2836-rpi-2-b.dts | 2 +-
arch/arm/boot/dts/bcm2836-rpi.dtsi | 6 ++++
arch/arm/boot/dts/bcm2837-rpi-3-b-plus.dts | 2 +-
arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 2 +-
arch/arm/boot/dts/bcm2837-rpi-cm3.dtsi | 2 +-
.../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++-
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 36 ++++++++++++++++------
.../vc04_services/interface/vchiq_arm/vchiq_arm.h | 5 +++
10 files changed, 48 insertions(+), 18 deletions(-)
create mode 100644 arch/arm/boot/dts/bcm2836-rpi.dtsi
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/4] staging/vc04_services: Use correct cache line size
2018-09-14 13:22 [PATCH v2 0/4] Improve VCHIQ cache line size handling Phil Elwell
@ 2018-09-14 13:22 ` Phil Elwell
2018-09-14 15:22 ` Phil Elwell
2018-09-14 17:03 ` Florian Fainelli
2018-09-14 13:22 ` [PATCH v2 2/4] dt-bindings: soc: Document "brcm,bcm2836-vchiq" Phil Elwell
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Phil Elwell @ 2018-09-14 13:22 UTC (permalink / raw)
To: Rob Herring, Stefan Wahren, Greg Kroah-Hartman, Phil Elwell,
devicetree, linux-rpi-kernel, Russell King, Arnd Bergmann,
linux-arm-kernel, bcm-kernel-feedback-list, devel
Use the compatible string in the DTB to select the correct cache line
size for the SoC - 32 for BCM2835, and 64 for BCM2836 and BCM2837.
Signed-off-by: Phil Elwell <phil@raspberrypi.org>
---
.../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++-
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 36 ++++++++++++++++------
.../vc04_services/interface/vchiq_arm/vchiq_arm.h | 5 +++
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index e767209..83d740f 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -109,7 +109,8 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
{
struct device *dev = &pdev->dev;
- struct rpi_firmware *fw = platform_get_drvdata(pdev);
+ struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
+ struct rpi_firmware *fw = drvdata->fw;
VCHIQ_SLOT_ZERO_T *vchiq_slot_zero;
struct resource *res;
void *slot_mem;
@@ -127,6 +128,7 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
if (err < 0)
return err;
+ g_cache_line_size = drvdata->cache_line_size;
g_fragments_size = 2 * g_cache_line_size;
/* Allocate space for the channels in coherent memory */
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index bc05c69..b2ae9259 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -170,6 +170,14 @@ static struct device *vchiq_dev;
static DEFINE_SPINLOCK(msg_queue_spinlock);
static struct platform_device *bcm2835_camera;
+static struct vchiq_drvdata bcm2835_drvdata = {
+ .cache_line_size = 32,
+};
+
+static struct vchiq_drvdata bcm2836_drvdata = {
+ .cache_line_size = 64,
+};
+
static const char *const ioctl_names[] = {
"CONNECT",
"SHUTDOWN",
@@ -3573,12 +3581,26 @@ void vchiq_platform_conn_state_changed(VCHIQ_STATE_T *state,
}
}
+static const struct of_device_id vchiq_of_match[] = {
+ { .compatible = "brcm,bcm2835-vchiq", .data = &bcm2835_drvdata },
+ { .compatible = "brcm,bcm2836-vchiq", .data = &bcm2836_drvdata },
+ {},
+};
+MODULE_DEVICE_TABLE(of, vchiq_of_match);
+
static int vchiq_probe(struct platform_device *pdev)
{
struct device_node *fw_node;
- struct rpi_firmware *fw;
+ const struct of_device_id *of_id;
+ struct vchiq_drvdata *drvdata;
int err;
+ snd_rpi_simple.dev = &pdev->dev;
+ of_id = of_match_node(vchiq_of_match, pdev->dev.of_node);
+ drvdata = of_id->data;
+ if (!drvdata)
+ return -EINVAL;
+
fw_node = of_find_compatible_node(NULL, NULL,
"raspberrypi,bcm2835-firmware");
if (!fw_node) {
@@ -3586,12 +3608,12 @@ static int vchiq_probe(struct platform_device *pdev)
return -ENOENT;
}
- fw = rpi_firmware_get(fw_node);
+ drvdata->fw = rpi_firmware_get(fw_node);
of_node_put(fw_node);
- if (!fw)
+ if (!drvdata->fw)
return -EPROBE_DEFER;
- platform_set_drvdata(pdev, fw);
+ platform_set_drvdata(pdev, drvdata);
err = vchiq_platform_init(pdev, &g_state);
if (err != 0)
@@ -3661,12 +3683,6 @@ static int vchiq_remove(struct platform_device *pdev)
return 0;
}
-static const struct of_device_id vchiq_of_match[] = {
- { .compatible = "brcm,bcm2835-vchiq", },
- {},
-};
-MODULE_DEVICE_TABLE(of, vchiq_of_match);
-
static struct platform_driver vchiq_driver = {
.driver = {
.name = "bcm2835_vchiq",
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
index 40bb0c6..2f3ebc9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -123,6 +123,11 @@ typedef struct vchiq_arm_state_struct {
} VCHIQ_ARM_STATE_T;
+struct vchiq_drvdata {
+ const unsigned int cache_line_size;
+ struct rpi_firmware *fw;
+};
+
extern int vchiq_arm_log_level;
extern int vchiq_susp_log_level;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] dt-bindings: soc: Document "brcm,bcm2836-vchiq"
2018-09-14 13:22 [PATCH v2 0/4] Improve VCHIQ cache line size handling Phil Elwell
2018-09-14 13:22 ` [PATCH v2 1/4] staging/vc04_services: Use correct cache line size Phil Elwell
@ 2018-09-14 13:22 ` Phil Elwell
2018-09-14 13:22 ` [PATCH v2 3/4] ARM: dts: bcm283x: Correct vchiq compatible string Phil Elwell
2018-09-14 13:22 ` [PATCH v2 4/4] ARM: dts: bcm283x: Correct mailbox register sizes Phil Elwell
3 siblings, 0 replies; 11+ messages in thread
From: Phil Elwell @ 2018-09-14 13:22 UTC (permalink / raw)
To: Rob Herring, Stefan Wahren, Greg Kroah-Hartman, Phil Elwell,
devicetree, linux-rpi-kernel, Russell King, Arnd Bergmann,
linux-arm-kernel, bcm-kernel-feedback-list, devel
"brcm,bcm2836-vchiq" should be used on BCM2836 and BCM2837 to ensure
correct operation.
Signed-off-by: Phil Elwell <phil@raspberrypi.org>
---
Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-vchiq.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-vchiq.txt b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-vchiq.txt
index 8dd7b3a..f331316 100644
--- a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-vchiq.txt
+++ b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-vchiq.txt
@@ -2,7 +2,8 @@ Broadcom VCHIQ firmware services
Required properties:
-- compatible: Should be "brcm,bcm2835-vchiq"
+- compatible: Should be "brcm,bcm2835-vchiq" on BCM2835, otherwise
+ "brcm,bcm2836-vchiq".
- reg: Physical base address and length of the doorbell register pair
- interrupts: The interrupt number
See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] ARM: dts: bcm283x: Correct vchiq compatible string
2018-09-14 13:22 [PATCH v2 0/4] Improve VCHIQ cache line size handling Phil Elwell
2018-09-14 13:22 ` [PATCH v2 1/4] staging/vc04_services: Use correct cache line size Phil Elwell
2018-09-14 13:22 ` [PATCH v2 2/4] dt-bindings: soc: Document "brcm,bcm2836-vchiq" Phil Elwell
@ 2018-09-14 13:22 ` Phil Elwell
2018-09-14 13:22 ` [PATCH v2 4/4] ARM: dts: bcm283x: Correct mailbox register sizes Phil Elwell
3 siblings, 0 replies; 11+ messages in thread
From: Phil Elwell @ 2018-09-14 13:22 UTC (permalink / raw)
To: Rob Herring, Stefan Wahren, Greg Kroah-Hartman, Phil Elwell,
devicetree, linux-rpi-kernel, Russell King, Arnd Bergmann,
linux-arm-kernel, bcm-kernel-feedback-list, devel
To allow VCHIQ to determine the correct cache line size, use the new
"brcm,bcm2836-vchiq" compatible string on BCM2836 and BCM2837.
Signed-off-by: Phil Elwell <phil@raspberrypi.org>
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 2 +-
arch/arm/boot/dts/bcm2836-rpi-2-b.dts | 2 +-
arch/arm/boot/dts/bcm2836-rpi.dtsi | 6 ++++++
arch/arm/boot/dts/bcm2837-rpi-3-b-plus.dts | 2 +-
arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 2 +-
arch/arm/boot/dts/bcm2837-rpi-cm3.dtsi | 2 +-
6 files changed, 11 insertions(+), 5 deletions(-)
create mode 100644 arch/arm/boot/dts/bcm2836-rpi.dtsi
diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index cb2d6d7..215d8cc 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -30,7 +30,7 @@
#power-domain-cells = <1>;
};
- mailbox@7e00b840 {
+ vchiq: mailbox@7e00b840 {
compatible = "brcm,bcm2835-vchiq";
reg = <0x7e00b840 0xf>;
interrupts = <0 2>;
diff --git a/arch/arm/boot/dts/bcm2836-rpi-2-b.dts b/arch/arm/boot/dts/bcm2836-rpi-2-b.dts
index 2fef70a..ac4408b 100644
--- a/arch/arm/boot/dts/bcm2836-rpi-2-b.dts
+++ b/arch/arm/boot/dts/bcm2836-rpi-2-b.dts
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/dts-v1/;
#include "bcm2836.dtsi"
-#include "bcm2835-rpi.dtsi"
+#include "bcm2836-rpi.dtsi"
#include "bcm283x-rpi-smsc9514.dtsi"
#include "bcm283x-rpi-usb-host.dtsi"
diff --git a/arch/arm/boot/dts/bcm2836-rpi.dtsi b/arch/arm/boot/dts/bcm2836-rpi.dtsi
new file mode 100644
index 0000000..c4c858b
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2836-rpi.dtsi
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "bcm2835-rpi.dtsi"
+
+&vchiq {
+ compatible = "brcm,bcm2836-vchiq", "brcm,bcm2835-vchiq";
+};
diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b-plus.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b-plus.dts
index 4adb85e..eca36e3 100644
--- a/arch/arm/boot/dts/bcm2837-rpi-3-b-plus.dts
+++ b/arch/arm/boot/dts/bcm2837-rpi-3-b-plus.dts
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/dts-v1/;
#include "bcm2837.dtsi"
-#include "bcm2835-rpi.dtsi"
+#include "bcm2836-rpi.dtsi"
#include "bcm283x-rpi-lan7515.dtsi"
#include "bcm283x-rpi-usb-host.dtsi"
diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
index c318bcb..a0ba0f6 100644
--- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
+++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/dts-v1/;
#include "bcm2837.dtsi"
-#include "bcm2835-rpi.dtsi"
+#include "bcm2836-rpi.dtsi"
#include "bcm283x-rpi-smsc9514.dtsi"
#include "bcm283x-rpi-usb-host.dtsi"
diff --git a/arch/arm/boot/dts/bcm2837-rpi-cm3.dtsi b/arch/arm/boot/dts/bcm2837-rpi-cm3.dtsi
index 7b7ab6a..4a89a18 100644
--- a/arch/arm/boot/dts/bcm2837-rpi-cm3.dtsi
+++ b/arch/arm/boot/dts/bcm2837-rpi-cm3.dtsi
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/dts-v1/;
#include "bcm2837.dtsi"
-#include "bcm2835-rpi.dtsi"
+#include "bcm2836-rpi.dtsi"
/ {
memory {
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] ARM: dts: bcm283x: Correct mailbox register sizes
2018-09-14 13:22 [PATCH v2 0/4] Improve VCHIQ cache line size handling Phil Elwell
` (2 preceding siblings ...)
2018-09-14 13:22 ` [PATCH v2 3/4] ARM: dts: bcm283x: Correct vchiq compatible string Phil Elwell
@ 2018-09-14 13:22 ` Phil Elwell
2018-09-14 17:01 ` Florian Fainelli
3 siblings, 1 reply; 11+ messages in thread
From: Phil Elwell @ 2018-09-14 13:22 UTC (permalink / raw)
To: Rob Herring, Stefan Wahren, Greg Kroah-Hartman, Phil Elwell,
devicetree, linux-rpi-kernel, Russell King, Arnd Bergmann,
linux-arm-kernel, bcm-kernel-feedback-list, devel
The size field in a Device Tree "reg" property is encoded in bytes, not
words.
Signed-off-by: Phil Elwell <phil@raspberrypi.org>
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 215d8cc..29f970f 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -32,7 +32,7 @@
vchiq: mailbox@7e00b840 {
compatible = "brcm,bcm2835-vchiq";
- reg = <0x7e00b840 0xf>;
+ reg = <0x7e00b840 0x3c>;
interrupts = <0 2>;
};
};
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size
2018-09-14 13:22 ` [PATCH v2 1/4] staging/vc04_services: Use correct cache line size Phil Elwell
@ 2018-09-14 15:22 ` Phil Elwell
2018-09-14 16:46 ` Stefan Wahren
2018-09-14 17:03 ` Florian Fainelli
1 sibling, 1 reply; 11+ messages in thread
From: Phil Elwell @ 2018-09-14 15:22 UTC (permalink / raw)
To: Rob Herring, Stefan Wahren, Greg Kroah-Hartman, devicetree,
linux-rpi-kernel, Russell King, Arnd Bergmann, linux-arm-kernel,
bcm-kernel-feedback-list, devel
On 14/09/2018 14:22, Phil Elwell wrote:
> Use the compatible string in the DTB to select the correct cache line
> size for the SoC - 32 for BCM2835, and 64 for BCM2836 and BCM2837.
>
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> ---
> .../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++-
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 36 ++++++++++++++++------
> .../vc04_services/interface/vchiq_arm/vchiq_arm.h | 5 +++
> 3 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index e767209..83d740f 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -109,7 +109,8 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
> int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
> {
> struct device *dev = &pdev->dev;
> - struct rpi_firmware *fw = platform_get_drvdata(pdev);
> + struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> + struct rpi_firmware *fw = drvdata->fw;
> VCHIQ_SLOT_ZERO_T *vchiq_slot_zero;
> struct resource *res;
> void *slot_mem;
> @@ -127,6 +128,7 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
> if (err < 0)
> return err;
>
> + g_cache_line_size = drvdata->cache_line_size;
> g_fragments_size = 2 * g_cache_line_size;
>
> /* Allocate space for the channels in coherent memory */
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index bc05c69..b2ae9259 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -170,6 +170,14 @@ static struct device *vchiq_dev;
> static DEFINE_SPINLOCK(msg_queue_spinlock);
> static struct platform_device *bcm2835_camera;
>
> +static struct vchiq_drvdata bcm2835_drvdata = {
> + .cache_line_size = 32,
> +};
> +
> +static struct vchiq_drvdata bcm2836_drvdata = {
> + .cache_line_size = 64,
> +};
> +
> static const char *const ioctl_names[] = {
> "CONNECT",
> "SHUTDOWN",
> @@ -3573,12 +3581,26 @@ void vchiq_platform_conn_state_changed(VCHIQ_STATE_T *state,
> }
> }
>
> +static const struct of_device_id vchiq_of_match[] = {
> + { .compatible = "brcm,bcm2835-vchiq", .data = &bcm2835_drvdata },
> + { .compatible = "brcm,bcm2836-vchiq", .data = &bcm2836_drvdata },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, vchiq_of_match);
> +
> static int vchiq_probe(struct platform_device *pdev)
> {
> struct device_node *fw_node;
> - struct rpi_firmware *fw;
> + const struct of_device_id *of_id;
> + struct vchiq_drvdata *drvdata;
> int err;
>
> + snd_rpi_simple.dev = &pdev->dev;
> + of_id = of_match_node(vchiq_of_match, pdev->dev.of_node);
> + drvdata = of_id->data;
> + if (!drvdata)
> + return -EINVAL;
> +
> fw_node = of_find_compatible_node(NULL, NULL,
> "raspberrypi,bcm2835-firmware");
> if (!fw_node) {
> @@ -3586,12 +3608,12 @@ static int vchiq_probe(struct platform_device *pdev)
> return -ENOENT;
> }
>
> - fw = rpi_firmware_get(fw_node);
> + drvdata->fw = rpi_firmware_get(fw_node);
> of_node_put(fw_node);
> - if (!fw)
> + if (!drvdata->fw)
> return -EPROBE_DEFER;
>
> - platform_set_drvdata(pdev, fw);
> + platform_set_drvdata(pdev, drvdata);
>
> err = vchiq_platform_init(pdev, &g_state);
> if (err != 0)
> @@ -3661,12 +3683,6 @@ static int vchiq_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id vchiq_of_match[] = {
> - { .compatible = "brcm,bcm2835-vchiq", },
> - {},
> -};
> -MODULE_DEVICE_TABLE(of, vchiq_of_match);
> -
> static struct platform_driver vchiq_driver = {
> .driver = {
> .name = "bcm2835_vchiq",
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
> index 40bb0c6..2f3ebc9 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
> @@ -123,6 +123,11 @@ typedef struct vchiq_arm_state_struct {
>
> } VCHIQ_ARM_STATE_T;
>
> +struct vchiq_drvdata {
> + const unsigned int cache_line_size;
> + struct rpi_firmware *fw;
> +};
> +
> extern int vchiq_arm_log_level;
> extern int vchiq_susp_log_level;
>
>
This version doesn't compile (wrong defconfig used when building), but any criticism of the
approach before v3 is welcome.
Phil
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size
2018-09-14 15:22 ` Phil Elwell
@ 2018-09-14 16:46 ` Stefan Wahren
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Wahren @ 2018-09-14 16:46 UTC (permalink / raw)
To: Phil Elwell, Rob Herring, Greg Kroah-Hartman, devicetree,
linux-rpi-kernel, Russell King, Arnd Bergmann, linux-arm-kernel,
bcm-kernel-feedback-list, devel
Hi Phil,
> Phil Elwell <phil@raspberrypi.org> hat am 14. September 2018 um 17:22 geschrieben:
>
>
> On 14/09/2018 14:22, Phil Elwell wrote:
> > Use the compatible string in the DTB to select the correct cache line
> > size for the SoC - 32 for BCM2835, and 64 for BCM2836 and BCM2837.
> >
> > Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> > ---
> > .../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++-
> > .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 36 ++++++++++++++++------
> > .../vc04_services/interface/vchiq_arm/vchiq_arm.h | 5 +++
> > 3 files changed, 34 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > index e767209..83d740f 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > @@ -109,7 +109,8 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
> > int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
> > {
> > struct device *dev = &pdev->dev;
> > - struct rpi_firmware *fw = platform_get_drvdata(pdev);
> > + struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> > + struct rpi_firmware *fw = drvdata->fw;
> > VCHIQ_SLOT_ZERO_T *vchiq_slot_zero;
> > struct resource *res;
> > void *slot_mem;
> > @@ -127,6 +128,7 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
> > if (err < 0)
> > return err;
> >
> > + g_cache_line_size = drvdata->cache_line_size;
> > g_fragments_size = 2 * g_cache_line_size;
> >
> > /* Allocate space for the channels in coherent memory */
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index bc05c69..b2ae9259 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -170,6 +170,14 @@ static struct device *vchiq_dev;
> > static DEFINE_SPINLOCK(msg_queue_spinlock);
> > static struct platform_device *bcm2835_camera;
> >
> > +static struct vchiq_drvdata bcm2835_drvdata = {
> > + .cache_line_size = 32,
> > +};
> > +
> > +static struct vchiq_drvdata bcm2836_drvdata = {
> > + .cache_line_size = 64,
> > +};
> > +
> > static const char *const ioctl_names[] = {
> > "CONNECT",
> > "SHUTDOWN",
> > @@ -3573,12 +3581,26 @@ void vchiq_platform_conn_state_changed(VCHIQ_STATE_T *state,
> > }
> > }
> >
> > +static const struct of_device_id vchiq_of_match[] = {
> > + { .compatible = "brcm,bcm2835-vchiq", .data = &bcm2835_drvdata },
> > + { .compatible = "brcm,bcm2836-vchiq", .data = &bcm2836_drvdata },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, vchiq_of_match);
> > +
> > static int vchiq_probe(struct platform_device *pdev)
> > {
> > struct device_node *fw_node;
> > - struct rpi_firmware *fw;
> > + const struct of_device_id *of_id;
> > + struct vchiq_drvdata *drvdata;
> > int err;
> >
> > + snd_rpi_simple.dev = &pdev->dev;
> > + of_id = of_match_node(vchiq_of_match, pdev->dev.of_node);
> > + drvdata = of_id->data;
> > + if (!drvdata)
> > + return -EINVAL;
> > +
> > fw_node = of_find_compatible_node(NULL, NULL,
> > "raspberrypi,bcm2835-firmware");
> > if (!fw_node) {
> > @@ -3586,12 +3608,12 @@ static int vchiq_probe(struct platform_device *pdev)
> > return -ENOENT;
> > }
> >
> > - fw = rpi_firmware_get(fw_node);
> > + drvdata->fw = rpi_firmware_get(fw_node);
> > of_node_put(fw_node);
> > - if (!fw)
> > + if (!drvdata->fw)
> > return -EPROBE_DEFER;
> >
> > - platform_set_drvdata(pdev, fw);
> > + platform_set_drvdata(pdev, drvdata);
> >
> > err = vchiq_platform_init(pdev, &g_state);
> > if (err != 0)
> > @@ -3661,12 +3683,6 @@ static int vchiq_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -static const struct of_device_id vchiq_of_match[] = {
> > - { .compatible = "brcm,bcm2835-vchiq", },
> > - {},
> > -};
> > -MODULE_DEVICE_TABLE(of, vchiq_of_match);
> > -
> > static struct platform_driver vchiq_driver = {
> > .driver = {
> > .name = "bcm2835_vchiq",
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
> > index 40bb0c6..2f3ebc9 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
> > @@ -123,6 +123,11 @@ typedef struct vchiq_arm_state_struct {
> >
> > } VCHIQ_ARM_STATE_T;
> >
> > +struct vchiq_drvdata {
> > + const unsigned int cache_line_size;
> > + struct rpi_firmware *fw;
> > +};
> > +
> > extern int vchiq_arm_log_level;
> > extern int vchiq_susp_log_level;
> >
> >
>
> This version doesn't compile (wrong defconfig used when building), but any criticism of the
> approach before v3 is welcome.
no need to hurry, my pull requests for 4.20 are already out. Please take the time to test this properly.
Patch 2-4 are:
Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>
> Phil
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] ARM: dts: bcm283x: Correct mailbox register sizes
2018-09-14 13:22 ` [PATCH v2 4/4] ARM: dts: bcm283x: Correct mailbox register sizes Phil Elwell
@ 2018-09-14 17:01 ` Florian Fainelli
0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2018-09-14 17:01 UTC (permalink / raw)
To: Phil Elwell, Rob Herring, Stefan Wahren, Greg Kroah-Hartman,
devicetree, linux-rpi-kernel, Russell King, Arnd Bergmann,
linux-arm-kernel, bcm-kernel-feedback-list, devel
On 09/14/2018 06:22 AM, Phil Elwell wrote:
> The size field in a Device Tree "reg" property is encoded in bytes, not
> words.
>
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
This should probably have a:
Fixes: 614fa22119d6 ("ARM: dts: bcm2835: Add VCHIQ node to the Raspberry
Pi boards. (v3)")
as well?
> ---
> arch/arm/boot/dts/bcm2835-rpi.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> index 215d8cc..29f970f 100644
> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> @@ -32,7 +32,7 @@
>
> vchiq: mailbox@7e00b840 {
> compatible = "brcm,bcm2835-vchiq";
> - reg = <0x7e00b840 0xf>;
> + reg = <0x7e00b840 0x3c>;
> interrupts = <0 2>;
> };
> };
>
--
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size
2018-09-14 13:22 ` [PATCH v2 1/4] staging/vc04_services: Use correct cache line size Phil Elwell
2018-09-14 15:22 ` Phil Elwell
@ 2018-09-14 17:03 ` Florian Fainelli
2018-09-14 18:12 ` Phil Elwell
1 sibling, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2018-09-14 17:03 UTC (permalink / raw)
To: Phil Elwell, Rob Herring, Stefan Wahren, Greg Kroah-Hartman,
devicetree, linux-rpi-kernel, Russell King, Arnd Bergmann,
linux-arm-kernel, bcm-kernel-feedback-list, devel
On 09/14/2018 06:22 AM, Phil Elwell wrote:
> Use the compatible string in the DTB to select the correct cache line
> size for the SoC - 32 for BCM2835, and 64 for BCM2836 and BCM2837.
>
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> ---
[snip]
> @@ -170,6 +170,14 @@ static struct device *vchiq_dev;
> static DEFINE_SPINLOCK(msg_queue_spinlock);
> static struct platform_device *bcm2835_camera;
>
> +static struct vchiq_drvdata bcm2835_drvdata = {
> + .cache_line_size = 32,
> +};
> +
> +static struct vchiq_drvdata bcm2836_drvdata = {
> + .cache_line_size = 64,
> +};
Those two structures could probably be marked const. Other than that,
the approach definitively looks good to me.
--
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size
2018-09-14 17:03 ` Florian Fainelli
@ 2018-09-14 18:12 ` Phil Elwell
2018-09-14 18:20 ` Florian Fainelli
0 siblings, 1 reply; 11+ messages in thread
From: Phil Elwell @ 2018-09-14 18:12 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Stefan Wahren, Greg Kroah-Hartman,
devicetree, linux-rpi-kernel, Russell King, Arnd Bergmann,
linux-arm-kernel, bcm-kernel-feedback-list, devel
On 14/09/2018 18:03, Florian Fainelli wrote:
> On 09/14/2018 06:22 AM, Phil Elwell wrote:
>> Use the compatible string in the DTB to select the correct cache line
>> size for the SoC - 32 for BCM2835, and 64 for BCM2836 and BCM2837.
>>
>> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
>> ---
>
> [snip]
>
>> @@ -170,6 +170,14 @@ static struct device *vchiq_dev;
>> static DEFINE_SPINLOCK(msg_queue_spinlock);
>> static struct platform_device *bcm2835_camera;
>>
>> +static struct vchiq_drvdata bcm2835_drvdata = {
>> + .cache_line_size = 32,
>> +};
>> +
>> +static struct vchiq_drvdata bcm2836_drvdata = {
>> + .cache_line_size = 64,
>> +};
>
> Those two structures could probably be marked const. Other than that,
> the approach definitively looks good to me.
The mutability is intentional - the structure pointer to the firmware is also
stored there. This isn't the only piece of mutable static data in a driver
that will only have a single instance, so allocating and initialising a
per-instance structure seems needlessly complicated.
Thanks for the feedback.
Phil
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size
2018-09-14 18:12 ` Phil Elwell
@ 2018-09-14 18:20 ` Florian Fainelli
0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2018-09-14 18:20 UTC (permalink / raw)
To: Phil Elwell, Florian Fainelli, Rob Herring, Stefan Wahren,
Greg Kroah-Hartman, devicetree, linux-rpi-kernel, Russell King,
Arnd Bergmann, linux-arm-kernel, bcm-kernel-feedback-list, devel
On 09/14/2018 11:12 AM, Phil Elwell wrote:
> On 14/09/2018 18:03, Florian Fainelli wrote:
>> On 09/14/2018 06:22 AM, Phil Elwell wrote:
>>> Use the compatible string in the DTB to select the correct cache line
>>> size for the SoC - 32 for BCM2835, and 64 for BCM2836 and BCM2837.
>>>
>>> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
>>> ---
>>
>> [snip]
>>
>>> @@ -170,6 +170,14 @@ static struct device *vchiq_dev;
>>> static DEFINE_SPINLOCK(msg_queue_spinlock);
>>> static struct platform_device *bcm2835_camera;
>>> +static struct vchiq_drvdata bcm2835_drvdata = {
>>> + .cache_line_size = 32,
>>> +};
>>> +
>>> +static struct vchiq_drvdata bcm2836_drvdata = {
>>> + .cache_line_size = 64,
>>> +};
>>
>> Those two structures could probably be marked const. Other than that,
>> the approach definitively looks good to me.
>
> The mutability is intentional - the structure pointer to the firmware is
> also
> stored there. This isn't the only piece of mutable static data in a driver
> that will only have a single instance, so allocating and initialising a
> per-instance structure seems needlessly complicated.
Fair enough, thanks for the explanation.
--
Florian
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-09-14 18:20 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-14 13:22 [PATCH v2 0/4] Improve VCHIQ cache line size handling Phil Elwell
2018-09-14 13:22 ` [PATCH v2 1/4] staging/vc04_services: Use correct cache line size Phil Elwell
2018-09-14 15:22 ` Phil Elwell
2018-09-14 16:46 ` Stefan Wahren
2018-09-14 17:03 ` Florian Fainelli
2018-09-14 18:12 ` Phil Elwell
2018-09-14 18:20 ` Florian Fainelli
2018-09-14 13:22 ` [PATCH v2 2/4] dt-bindings: soc: Document "brcm,bcm2836-vchiq" Phil Elwell
2018-09-14 13:22 ` [PATCH v2 3/4] ARM: dts: bcm283x: Correct vchiq compatible string Phil Elwell
2018-09-14 13:22 ` [PATCH v2 4/4] ARM: dts: bcm283x: Correct mailbox register sizes Phil Elwell
2018-09-14 17:01 ` Florian Fainelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).