* [PATCH 0/2] Remove global struct vchiq_drvdata variables
@ 2019-11-11 17:14 Marcelo Diop-Gonzalez
2019-11-11 17:14 ` [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init() Marcelo Diop-Gonzalez
2019-11-11 17:14 ` [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata Marcelo Diop-Gonzalez
0 siblings, 2 replies; 13+ messages in thread
From: Marcelo Diop-Gonzalez @ 2019-11-11 17:14 UTC (permalink / raw)
To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel
These appear to be populated once in vchiq_probe() and then read
once in vchiq_platform_init(), so by moving the logic to that
function we can remove some (small amount of) global data.
Marcelo Diop-Gonzalez (2):
staging: vchiq: Move retrieval of rpi_firmware to
vchiq_platform_init()
staging: vchiq: Remove global bcm*_drvdata
.../interface/vchiq_arm/vchiq_2835_arm.c | 18 ++++++++--
.../interface/vchiq_arm/vchiq_arm.c | 34 ++-----------------
.../interface/vchiq_arm/vchiq_arm.h | 5 ---
3 files changed, 17 insertions(+), 40 deletions(-)
--
2.20.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init()
2019-11-11 17:14 [PATCH 0/2] Remove global struct vchiq_drvdata variables Marcelo Diop-Gonzalez
@ 2019-11-11 17:14 ` Marcelo Diop-Gonzalez
2019-11-12 23:09 ` Greg KH
2019-11-13 18:17 ` Dan Carpenter
2019-11-11 17:14 ` [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata Marcelo Diop-Gonzalez
1 sibling, 2 replies; 13+ messages in thread
From: Marcelo Diop-Gonzalez @ 2019-11-11 17:14 UTC (permalink / raw)
To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel
This allows the removal of the struct rpi_firmware* member
from struct vchiq_drvdata.
Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
---
.../interface/vchiq_arm/vchiq_2835_arm.c | 14 +++++++++++++-
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 --------------
.../vc04_services/interface/vchiq_arm/vchiq_arm.h | 1 -
3 files changed, 13 insertions(+), 16 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 8dc730cfe7a6..5ac88be9496b 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
@@ -79,7 +79,8 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
{
struct device *dev = &pdev->dev;
struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
- struct rpi_firmware *fw = drvdata->fw;
+ struct device_node *fw_node;
+ struct rpi_firmware *fw;
struct vchiq_slot_zero *vchiq_slot_zero;
struct resource *res;
void *slot_mem;
@@ -88,6 +89,17 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
int slot_mem_size, frag_mem_size;
int err, irq, i;
+ fw_node = of_find_compatible_node(NULL, NULL,
+ "raspberrypi,bcm2835-firmware");
+ if (!fw_node) {
+ dev_err(&pdev->dev, "Missing firmware node\n");
+ return -ENOENT;
+ }
+ fw = rpi_firmware_get(fw_node);
+ of_node_put(fw_node);
+ if (!fw)
+ return -EPROBE_DEFER;
+
/*
* VCHI messages between the CPU and firmware use
* 32-bit bus addresses.
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 b1595b13dea8..c666c8b5eda2 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -3191,7 +3191,6 @@ vchiq_register_child(struct platform_device *pdev, const char *name)
static int vchiq_probe(struct platform_device *pdev)
{
- struct device_node *fw_node;
const struct of_device_id *of_id;
struct vchiq_drvdata *drvdata;
struct device *vchiq_dev;
@@ -3201,19 +3200,6 @@ static int vchiq_probe(struct platform_device *pdev)
drvdata = (struct vchiq_drvdata *)of_id->data;
if (!drvdata)
return -EINVAL;
-
- fw_node = of_find_compatible_node(NULL, NULL,
- "raspberrypi,bcm2835-firmware");
- if (!fw_node) {
- dev_err(&pdev->dev, "Missing firmware node\n");
- return -ENOENT;
- }
-
- drvdata->fw = rpi_firmware_get(fw_node);
- of_node_put(fw_node);
- if (!drvdata->fw)
- return -EPROBE_DEFER;
-
platform_set_drvdata(pdev, drvdata);
err = vchiq_platform_init(pdev, &g_state);
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 b424323e9613..e2cdfddaf02a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -97,7 +97,6 @@ struct vchiq_arm_state {
struct vchiq_drvdata {
const unsigned int cache_line_size;
- struct rpi_firmware *fw;
};
extern int vchiq_arm_log_level;
--
2.20.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata
2019-11-11 17:14 [PATCH 0/2] Remove global struct vchiq_drvdata variables Marcelo Diop-Gonzalez
2019-11-11 17:14 ` [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init() Marcelo Diop-Gonzalez
@ 2019-11-11 17:14 ` Marcelo Diop-Gonzalez
2019-11-13 18:18 ` Dan Carpenter
2019-11-13 18:40 ` Stefan Wahren
1 sibling, 2 replies; 13+ messages in thread
From: Marcelo Diop-Gonzalez @ 2019-11-11 17:14 UTC (permalink / raw)
To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel
Moving the DT node check to vchiq_platform_init()
removes the need for these.
Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
---
.../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++--
.../interface/vchiq_arm/vchiq_arm.c | 20 ++-----------------
.../interface/vchiq_arm/vchiq_arm.h | 4 ----
3 files changed, 4 insertions(+), 24 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 5ac88be9496b..b133b25c44af 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
@@ -78,7 +78,6 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
{
struct device *dev = &pdev->dev;
- struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
struct device_node *fw_node;
struct rpi_firmware *fw;
struct vchiq_slot_zero *vchiq_slot_zero;
@@ -109,7 +108,8 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
if (err < 0)
return err;
- g_cache_line_size = drvdata->cache_line_size;
+ if (of_device_is_compatible(dev->of_node, "brcm,bcm2836-vchiq"))
+ g_cache_line_size = 64;
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 c666c8b5eda2..cc753ba9c68c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -140,14 +140,6 @@ static DEFINE_SPINLOCK(msg_queue_spinlock);
static struct platform_device *bcm2835_camera;
static struct platform_device *bcm2835_audio;
-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",
@@ -3161,8 +3153,8 @@ void vchiq_platform_conn_state_changed(struct vchiq_state *state,
}
static const struct of_device_id vchiq_of_match[] = {
- { .compatible = "brcm,bcm2835-vchiq", .data = &bcm2835_drvdata },
- { .compatible = "brcm,bcm2836-vchiq", .data = &bcm2836_drvdata },
+ { .compatible = "brcm,bcm2835-vchiq"},
+ { .compatible = "brcm,bcm2836-vchiq"},
{},
};
MODULE_DEVICE_TABLE(of, vchiq_of_match);
@@ -3191,17 +3183,9 @@ vchiq_register_child(struct platform_device *pdev, const char *name)
static int vchiq_probe(struct platform_device *pdev)
{
- const struct of_device_id *of_id;
- struct vchiq_drvdata *drvdata;
struct device *vchiq_dev;
int err;
- of_id = of_match_node(vchiq_of_match, pdev->dev.of_node);
- drvdata = (struct vchiq_drvdata *)of_id->data;
- if (!drvdata)
- return -EINVAL;
- platform_set_drvdata(pdev, drvdata);
-
err = vchiq_platform_init(pdev, &g_state);
if (err)
goto failed_platform_init;
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 e2cdfddaf02a..71b97d8d90bd 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -95,10 +95,6 @@ struct vchiq_arm_state {
};
-struct vchiq_drvdata {
- const unsigned int cache_line_size;
-};
-
extern int vchiq_arm_log_level;
extern int vchiq_susp_log_level;
--
2.20.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init()
2019-11-11 17:14 ` [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init() Marcelo Diop-Gonzalez
@ 2019-11-12 23:09 ` Greg KH
2019-11-12 23:29 ` Marcelo Diop-Gonzalez
2019-11-13 20:29 ` Marcelo Diop-Gonzalez
2019-11-13 18:17 ` Dan Carpenter
1 sibling, 2 replies; 13+ messages in thread
From: Greg KH @ 2019-11-12 23:09 UTC (permalink / raw)
To: Marcelo Diop-Gonzalez; +Cc: devel, eric, wahrenst, linux-rpi-kernel
On Mon, Nov 11, 2019 at 12:14:23PM -0500, Marcelo Diop-Gonzalez wrote:
> This allows the removal of the struct rpi_firmware* member
> from struct vchiq_drvdata.
>
> Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
> ---
> .../interface/vchiq_arm/vchiq_2835_arm.c | 14 +++++++++++++-
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 --------------
> .../vc04_services/interface/vchiq_arm/vchiq_arm.h | 1 -
> 3 files changed, 13 insertions(+), 16 deletions(-)
This commit breaks the build:
In function ‘free_pagelist’,
inlined from ‘vchiq_complete_bulk’ at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:258:3:
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:588:4: warning: argument 2 null where non-null expected [-Wnonnull]
588 | memcpy((char *)kmap(pages[0]) +
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
589 | pagelist->offset,
| ~~~~~~~~~~~~~~~~~
590 | fragments,
| ~~~~~~~~~~
591 | head_bytes);
| ~~~~~~~~~~~
Please be more careful and at least test your changes before sending
them out :(
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init()
2019-11-12 23:09 ` Greg KH
@ 2019-11-12 23:29 ` Marcelo Diop-Gonzalez
2019-11-13 5:32 ` Greg KH
2019-11-13 20:29 ` Marcelo Diop-Gonzalez
1 sibling, 1 reply; 13+ messages in thread
From: Marcelo Diop-Gonzalez @ 2019-11-12 23:29 UTC (permalink / raw)
To: Greg KH; +Cc: devel, eric, Stefan Wahren, linux-rpi-kernel
On Tue, Nov 12, 2019 at 6:09 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Nov 11, 2019 at 12:14:23PM -0500, Marcelo Diop-Gonzalez wrote:
> > This allows the removal of the struct rpi_firmware* member
> > from struct vchiq_drvdata.
> >
> > Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
> > ---
> > .../interface/vchiq_arm/vchiq_2835_arm.c | 14 +++++++++++++-
> > .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 --------------
> > .../vc04_services/interface/vchiq_arm/vchiq_arm.h | 1 -
> > 3 files changed, 13 insertions(+), 16 deletions(-)
>
> This commit breaks the build:
>
> In function ‘free_pagelist’,
> inlined from ‘vchiq_complete_bulk’ at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:258:3:
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:588:4: warning: argument 2 null where non-null expected [-Wnonnull]
> 588 | memcpy((char *)kmap(pages[0]) +
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 589 | pagelist->offset,
> | ~~~~~~~~~~~~~~~~~
> 590 | fragments,
> | ~~~~~~~~~~
> 591 | head_bytes);
> | ~~~~~~~~~~~
>
> Please be more careful and at least test your changes before sending
> them out :(
Ah sorry :(((. Could you let me know what config you saw that error
with? I see no warnings/errors with bcm2835_defconfig.
>
> greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init()
2019-11-12 23:29 ` Marcelo Diop-Gonzalez
@ 2019-11-13 5:32 ` Greg KH
2019-11-13 14:08 ` Marcelo Diop-Gonzalez
0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2019-11-13 5:32 UTC (permalink / raw)
To: Marcelo Diop-Gonzalez; +Cc: devel, eric, Stefan Wahren, linux-rpi-kernel
On Tue, Nov 12, 2019 at 06:29:55PM -0500, Marcelo Diop-Gonzalez wrote:
> On Tue, Nov 12, 2019 at 6:09 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Nov 11, 2019 at 12:14:23PM -0500, Marcelo Diop-Gonzalez wrote:
> > > This allows the removal of the struct rpi_firmware* member
> > > from struct vchiq_drvdata.
> > >
> > > Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
> > > ---
> > > .../interface/vchiq_arm/vchiq_2835_arm.c | 14 +++++++++++++-
> > > .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 --------------
> > > .../vc04_services/interface/vchiq_arm/vchiq_arm.h | 1 -
> > > 3 files changed, 13 insertions(+), 16 deletions(-)
> >
> > This commit breaks the build:
> >
> > In function ‘free_pagelist’,
> > inlined from ‘vchiq_complete_bulk’ at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:258:3:
> > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:588:4: warning: argument 2 null where non-null expected [-Wnonnull]
> > 588 | memcpy((char *)kmap(pages[0]) +
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 589 | pagelist->offset,
> > | ~~~~~~~~~~~~~~~~~
> > 590 | fragments,
> > | ~~~~~~~~~~
> > 591 | head_bytes);
> > | ~~~~~~~~~~~
> >
> > Please be more careful and at least test your changes before sending
> > them out :(
>
> Ah sorry :(((. Could you let me know what config you saw that error
> with? I see no warnings/errors with bcm2835_defconfig.
I'm just building on x86-64, try 'allmodconfig' on your desktop and see
what happens.
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init()
2019-11-13 5:32 ` Greg KH
@ 2019-11-13 14:08 ` Marcelo Diop-Gonzalez
0 siblings, 0 replies; 13+ messages in thread
From: Marcelo Diop-Gonzalez @ 2019-11-13 14:08 UTC (permalink / raw)
To: Greg KH; +Cc: devel, eric, Stefan Wahren, linux-rpi-kernel
On Wed, Nov 13, 2019 at 12:32 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Nov 12, 2019 at 06:29:55PM -0500, Marcelo Diop-Gonzalez wrote:
> > On Tue, Nov 12, 2019 at 6:09 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Nov 11, 2019 at 12:14:23PM -0500, Marcelo Diop-Gonzalez wrote:
> > > > This allows the removal of the struct rpi_firmware* member
> > > > from struct vchiq_drvdata.
> > > >
> > > > Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
> > > > ---
> > > > .../interface/vchiq_arm/vchiq_2835_arm.c | 14 +++++++++++++-
> > > > .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 --------------
> > > > .../vc04_services/interface/vchiq_arm/vchiq_arm.h | 1 -
> > > > 3 files changed, 13 insertions(+), 16 deletions(-)
> > >
> > > This commit breaks the build:
> > >
> > > In function ‘free_pagelist’,
> > > inlined from ‘vchiq_complete_bulk’ at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:258:3:
> > > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:588:4: warning: argument 2 null where non-null expected [-Wnonnull]
> > > 588 | memcpy((char *)kmap(pages[0]) +
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 589 | pagelist->offset,
> > > | ~~~~~~~~~~~~~~~~~
> > > 590 | fragments,
> > > | ~~~~~~~~~~
> > > 591 | head_bytes);
> > > | ~~~~~~~~~~~
> > >
> > > Please be more careful and at least test your changes before sending
> > > them out :(
> >
> > Ah sorry :(((. Could you let me know what config you saw that error
> > with? I see no warnings/errors with bcm2835_defconfig.
>
> I'm just building on x86-64, try 'allmodconfig' on your desktop and see
> what happens.
>
> greg k-h
Ah yeah I see now, will fix.
-Marcelo
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init()
2019-11-11 17:14 ` [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init() Marcelo Diop-Gonzalez
2019-11-12 23:09 ` Greg KH
@ 2019-11-13 18:17 ` Dan Carpenter
2019-11-13 19:58 ` Marcelo Diop-Gonzalez
1 sibling, 1 reply; 13+ messages in thread
From: Dan Carpenter @ 2019-11-13 18:17 UTC (permalink / raw)
To: Marcelo Diop-Gonzalez; +Cc: devel, gregkh, linux-rpi-kernel, wahrenst, eric
On Mon, Nov 11, 2019 at 12:14:23PM -0500, Marcelo Diop-Gonzalez wrote:
> This allows the removal of the struct rpi_firmware* member
> from struct vchiq_drvdata.
>
Please don't start your commit message in the middle of a sentence.
It looks like this:
https://marc.info/?l=linux-driver-devel&m=157349280800959&w=2
> Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata
2019-11-11 17:14 ` [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata Marcelo Diop-Gonzalez
@ 2019-11-13 18:18 ` Dan Carpenter
2019-11-13 18:40 ` Stefan Wahren
1 sibling, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-11-13 18:18 UTC (permalink / raw)
To: Marcelo Diop-Gonzalez; +Cc: devel, gregkh, linux-rpi-kernel, wahrenst, eric
On Mon, Nov 11, 2019 at 12:14:24PM -0500, Marcelo Diop-Gonzalez wrote:
> @@ -109,7 +108,8 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> if (err < 0)
> return err;
>
> - g_cache_line_size = drvdata->cache_line_size;
> + if (of_device_is_compatible(dev->of_node, "brcm,bcm2836-vchiq"))
> + g_cache_line_size = 64;
Could you remove the initialization and do it like this:
if (of_device_is_compatible(dev->of_node, "brcm,bcm2836-vchiq"))
g_cache_line_size = 64;
else
g_cache_line_size = 32;
Maybe we could even just remove the global?
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata
2019-11-11 17:14 ` [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata Marcelo Diop-Gonzalez
2019-11-13 18:18 ` Dan Carpenter
@ 2019-11-13 18:40 ` Stefan Wahren
2019-11-13 19:56 ` Marcelo Diop-Gonzalez
1 sibling, 1 reply; 13+ messages in thread
From: Stefan Wahren @ 2019-11-13 18:40 UTC (permalink / raw)
To: Marcelo Diop-Gonzalez, gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel
Hello Marcelo,
Am 11.11.19 um 18:14 schrieb Marcelo Diop-Gonzalez:
> Moving the DT node check to vchiq_platform_init()
> removes the need for these.
this comment does match to your changes. Also i'm missing a why this is
necessary.
>
> Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
> ---
> .../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++--
> .../interface/vchiq_arm/vchiq_arm.c | 20 ++-----------------
> .../interface/vchiq_arm/vchiq_arm.h | 4 ----
> 3 files changed, 4 insertions(+), 24 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 5ac88be9496b..b133b25c44af 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
> @@ -78,7 +78,6 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
> int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> {
> struct device *dev = &pdev->dev;
> - struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> struct device_node *fw_node;
> struct rpi_firmware *fw;
> struct vchiq_slot_zero *vchiq_slot_zero;
> @@ -109,7 +108,8 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> if (err < 0)
> return err;
>
> - g_cache_line_size = drvdata->cache_line_size;
> + if (of_device_is_compatible(dev->of_node, "brcm,bcm2836-vchiq"))
> + g_cache_line_size = 64;
> 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 c666c8b5eda2..cc753ba9c68c 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -140,14 +140,6 @@ static DEFINE_SPINLOCK(msg_queue_spinlock);
> static struct platform_device *bcm2835_camera;
> static struct platform_device *bcm2835_audio;
>
> -static struct vchiq_drvdata bcm2835_drvdata = {
> - .cache_line_size = 32,
> -};
> -
> -static struct vchiq_drvdata bcm2836_drvdata = {
> - .cache_line_size = 64,
> -};
You are touching new code which was introduced to keep the driver easier
to maintain (e.g. add support for new SoCs like BCM2711 on Raspberry Pi 4).
So please keep the OF match data handling. But i'm happy to see the
platform data as a const.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata
2019-11-13 18:40 ` Stefan Wahren
@ 2019-11-13 19:56 ` Marcelo Diop-Gonzalez
0 siblings, 0 replies; 13+ messages in thread
From: Marcelo Diop-Gonzalez @ 2019-11-13 19:56 UTC (permalink / raw)
To: Stefan Wahren; +Cc: devel, Greg KH, linux-rpi-kernel, eric
On Wed, Nov 13, 2019 at 1:40 PM Stefan Wahren <wahrenst@gmx.net> wrote:
>
> Hello Marcelo,
>
> Am 11.11.19 um 18:14 schrieb Marcelo Diop-Gonzalez:
> > Moving the DT node check to vchiq_platform_init()
> > removes the need for these.
>
> this comment does match to your changes. Also i'm missing a why this is
> necessary.
>
> >
> > Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
> > ---
> > .../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++--
> > .../interface/vchiq_arm/vchiq_arm.c | 20 ++-----------------
> > .../interface/vchiq_arm/vchiq_arm.h | 4 ----
> > 3 files changed, 4 insertions(+), 24 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 5ac88be9496b..b133b25c44af 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
> > @@ -78,7 +78,6 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
> > int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > {
> > struct device *dev = &pdev->dev;
> > - struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> > struct device_node *fw_node;
> > struct rpi_firmware *fw;
> > struct vchiq_slot_zero *vchiq_slot_zero;
> > @@ -109,7 +108,8 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > if (err < 0)
> > return err;
> >
> > - g_cache_line_size = drvdata->cache_line_size;
> > + if (of_device_is_compatible(dev->of_node, "brcm,bcm2836-vchiq"))
> > + g_cache_line_size = 64;
> > 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 c666c8b5eda2..cc753ba9c68c 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -140,14 +140,6 @@ static DEFINE_SPINLOCK(msg_queue_spinlock);
> > static struct platform_device *bcm2835_camera;
> > static struct platform_device *bcm2835_audio;
> >
> > -static struct vchiq_drvdata bcm2835_drvdata = {
> > - .cache_line_size = 32,
> > -};
> > -
> > -static struct vchiq_drvdata bcm2836_drvdata = {
> > - .cache_line_size = 64,
> > -};
> You are touching new code which was introduced to keep the driver easier
> to maintain (e.g. add support for new SoCs like BCM2711 on Raspberry Pi 4).
>
> So please keep the OF match data handling. But i'm happy to see the
> platform data as a const.
>
Oh ok I see, didn't realize that was the reason for it. I think we
should just scrap this patchset in that case.
Thanks,
-Marcelo
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init()
2019-11-13 18:17 ` Dan Carpenter
@ 2019-11-13 19:58 ` Marcelo Diop-Gonzalez
0 siblings, 0 replies; 13+ messages in thread
From: Marcelo Diop-Gonzalez @ 2019-11-13 19:58 UTC (permalink / raw)
To: Dan Carpenter; +Cc: devel, Greg KH, linux-rpi-kernel, Stefan Wahren, eric
On Wed, Nov 13, 2019 at 1:19 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Mon, Nov 11, 2019 at 12:14:23PM -0500, Marcelo Diop-Gonzalez wrote:
> > This allows the removal of the struct rpi_firmware* member
> > from struct vchiq_drvdata.
> >
>
> Please don't start your commit message in the middle of a sentence.
>
> It looks like this:
> https://marc.info/?l=linux-driver-devel&m=157349280800959&w=2
Whoops. Thanks for the pointer.
>
>
> > Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
>
> regards,
> dan carpenter
-Marcelo
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init()
2019-11-12 23:09 ` Greg KH
2019-11-12 23:29 ` Marcelo Diop-Gonzalez
@ 2019-11-13 20:29 ` Marcelo Diop-Gonzalez
1 sibling, 0 replies; 13+ messages in thread
From: Marcelo Diop-Gonzalez @ 2019-11-13 20:29 UTC (permalink / raw)
To: Greg KH; +Cc: devel, eric, Stefan Wahren, linux-rpi-kernel
On Tue, Nov 12, 2019 at 6:09 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Nov 11, 2019 at 12:14:23PM -0500, Marcelo Diop-Gonzalez wrote:
> > This allows the removal of the struct rpi_firmware* member
> > from struct vchiq_drvdata.
> >
> > Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
> > ---
> > .../interface/vchiq_arm/vchiq_2835_arm.c | 14 +++++++++++++-
> > .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 --------------
> > .../vc04_services/interface/vchiq_arm/vchiq_arm.h | 1 -
> > 3 files changed, 13 insertions(+), 16 deletions(-)
>
> This commit breaks the build:
>
> In function ‘free_pagelist’,
> inlined from ‘vchiq_complete_bulk’ at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:258:3:
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:588:4: warning: argument 2 null where non-null expected [-Wnonnull]
> 588 | memcpy((char *)kmap(pages[0]) +
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 589 | pagelist->offset,
> | ~~~~~~~~~~~~~~~~~
> 590 | fragments,
> | ~~~~~~~~~~
> 591 | head_bytes);
> | ~~~~~~~~~~~
>
I think this may be an issue that will show up if anyone in the future
tries adding code to vchiq_platform_init() before g_fragments_base and
g_fragments_size are assigned-to that is guaranteed to return when
CONFIG_RASPBERRYPI_FIRMWARE is not set, since these make up the
fragments argument to the call to memcpy() in free_pagelist():
char *fragments = g_fragments_base +
(pagelist->type -
PAGELIST_READ_WITH_FRAGMENTS) *
g_fragments_size;
Adding the line "return -1;" to the beginning of vchiq_platform_init()
reproduces the warning, but adding it just after g_fragments_size is
assigned-to doesn't. Maybe worth fixing in a separate patch.
> Please be more careful and at least test your changes before sending
> them out :(
>
> greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-11-13 20:29 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-11 17:14 [PATCH 0/2] Remove global struct vchiq_drvdata variables Marcelo Diop-Gonzalez
2019-11-11 17:14 ` [PATCH 1/2] staging: vchiq: Move retrieval of rpi_firmware to vchiq_platform_init() Marcelo Diop-Gonzalez
2019-11-12 23:09 ` Greg KH
2019-11-12 23:29 ` Marcelo Diop-Gonzalez
2019-11-13 5:32 ` Greg KH
2019-11-13 14:08 ` Marcelo Diop-Gonzalez
2019-11-13 20:29 ` Marcelo Diop-Gonzalez
2019-11-13 18:17 ` Dan Carpenter
2019-11-13 19:58 ` Marcelo Diop-Gonzalez
2019-11-11 17:14 ` [PATCH 2/2] staging: vchiq: Remove global bcm*_drvdata Marcelo Diop-Gonzalez
2019-11-13 18:18 ` Dan Carpenter
2019-11-13 18:40 ` Stefan Wahren
2019-11-13 19:56 ` Marcelo Diop-Gonzalez
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).