[PATCH v2 0/4] Improve VCHIQ cache line size handling

[PATCH v2 0/4] Improve VCHIQ cache line size handling

[Phil Elwell]

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

[PATCH v2 1/4] staging/vc04_services: Use correct cache line size

[Phil Elwell]

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

Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size

[Phil Elwell]

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

Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size

[Stefan Wahren]

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

Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size

[Florian Fainelli]

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

Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size

[Phil Elwell]

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

Re: [PATCH v2 1/4] staging/vc04_services: Use correct cache line size

[Florian Fainelli]

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

[PATCH v2 2/4] dt-bindings: soc: Document "brcm,bcm2836-vchiq"

[Phil Elwell]

"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

[PATCH v2 3/4] ARM: dts: bcm283x: Correct vchiq compatible string

[Phil Elwell]

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

[PATCH v2 4/4] ARM: dts: bcm283x: Correct mailbox register sizes

[Phil Elwell]

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

Re: [PATCH v2 4/4] ARM: dts: bcm283x: Correct mailbox register sizes

[Florian Fainelli]

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