mbox series

[v6,0/6] staging: vc04_services: vchiq: Register devices with a custom bus_type

Message ID 20230120201104.606876-1-umang.jain@ideasonboard.com
Headers show
Series staging: vc04_services: vchiq: Register devices with a custom bus_type | expand

Message

Umang Jain Jan. 20, 2023, 8:10 p.m. UTC
This series just introduces five extra patches for dropping include
directives from Makefiles (suggested by Greg KH) and rebased.

The main patch (6/6) removes platform device/driver abuse and moves
things to standard device/driver model using a custom_bus. Specific
details are elaborated in the commit message.

The patch series is based on top of d514392f17fd (tag: next-20230120)
of linux-next.

Changes in v6:
- Split struct device and struct driver wrappers in vchiq_device.[ch]
- Move vchiq_bus_type definition to vchiq_device.[ch] as well
- return error on bus_register() failure
- drop dma_set_mask_and_coherent
- trivial variable name change

Changes in v5:
- Fixup missing "staging: " in commits' subject line
- No code changes from v4

Changes in v4: 
- Introduce patches to drop include directives from Makefile

Changes in v3:
- Rework entirely to replace platform devices/driver model

-v2:
https://lore.kernel.org/all/20221222191500.515795-1-umang.jain@ideasonboard.com/

-v1:
https://lore.kernel.org/all/20221220084404.19280-1-umang.jain@ideasonboard.com/

Umang Jain (6):
  staging: vc04_services: Drop __VCCOREVER__ remnants
  staging: vc04_services: bcm2835-audio: Drop include Makefile directive
  staging: vc04_services: bcm2835-camera: Drop include Makefile
    directive
  staging: vc04_services: vchiq-mmal: Drop include Makefile directive
  staging: vc04_services: interface: Drop include Makefile directive
  staging: vc04_services: vchiq: Register devices with a custom bus_type

 drivers/staging/vc04_services/Makefile        |   3 +-
 .../vc04_services/bcm2835-audio/Makefile      |   2 -
 .../vc04_services/bcm2835-audio/bcm2835.c     |  27 +++--
 .../vc04_services/bcm2835-audio/bcm2835.h     |   3 +-
 .../vc04_services/bcm2835-camera/Makefile     |   5 -
 .../bcm2835-camera/bcm2835-camera.c           |  35 +++---
 .../vc04_services/bcm2835-camera/controls.c   |   6 +-
 .../interface/vchiq_arm/vchiq_arm.c           |  52 +++++----
 .../interface/vchiq_arm/vchiq_core.h          |   2 +-
 .../interface/vchiq_arm/vchiq_device.c        | 102 ++++++++++++++++++
 .../interface/vchiq_arm/vchiq_device.h        |  39 +++++++
 .../interface/vchiq_arm/vchiq_ioctl.h         |   3 +-
 .../staging/vc04_services/vchiq-mmal/Makefile |   5 -
 .../vc04_services/vchiq-mmal/mmal-vchiq.c     |   2 +-
 14 files changed, 206 insertions(+), 80 deletions(-)
 create mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
 create mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.h

Comments

Stefan Wahren Jan. 22, 2023, 11:34 p.m. UTC | #1
Hi Umang,

Am 20.01.23 um 21:10 schrieb Umang Jain:
> This series just introduces five extra patches for dropping include
> directives from Makefiles (suggested by Greg KH) and rebased.
>
> The main patch (6/6) removes platform device/driver abuse and moves
> things to standard device/driver model using a custom_bus. Specific
> details are elaborated in the commit message.
>
> The patch series is based on top of d514392f17fd (tag: next-20230120)
> of linux-next.

applied this series on top of linux-next and build it with 
arm/multi_v7_defconfig plus the following:

CONFIG_BCM_VIDEOCORE=y
CONFIG_BCM2835_VCHIQ=m
CONFIG_VCHIQ_CDEV=y
CONFIG_SND_BCM2835=m
CONFIG_VIDEO_BCM2835=m
CONFIG_BCM2835_VCHIQ_MMAL=m

and the devices doesn't register on Raspberry Pi 3 B Plus:

[   25.523337] vchiq: module is from the staging directory, the quality 
is unknown, you have been warned.
[   25.541647] bcm2835_vchiq 3f00b840.mailbox: Failed to register 
bcm2835_audio vchiq device
[   25.553692] bcm2835_vchiq 3f00b840.mailbox: Failed to register 
bcm2835-camera vchiq device
Laurent Pinchart Jan. 23, 2023, 8:58 a.m. UTC | #2
On Mon, Jan 23, 2023 at 01:18:30PM +0530, Umang Jain wrote:
> Hi Stefan,
> 
> Thank for the testing.
> 
> On 1/23/23 5:04 AM, Stefan Wahren wrote:
> > Hi Umang,
> >
> > Am 20.01.23 um 21:10 schrieb Umang Jain:
> >> This series just introduces five extra patches for dropping include
> >> directives from Makefiles (suggested by Greg KH) and rebased.
> >>
> >> The main patch (6/6) removes platform device/driver abuse and moves
> >> things to standard device/driver model using a custom_bus. Specific
> >> details are elaborated in the commit message.
> >>
> >> The patch series is based on top of d514392f17fd (tag: next-20230120)
> >> of linux-next.
> >
> > applied this series on top of linux-next and build it with 
> > arm/multi_v7_defconfig plus the following:
> >
> > CONFIG_BCM_VIDEOCORE=y
> > CONFIG_BCM2835_VCHIQ=m
> > CONFIG_VCHIQ_CDEV=y
> > CONFIG_SND_BCM2835=m
> > CONFIG_VIDEO_BCM2835=m
> > CONFIG_BCM2835_VCHIQ_MMAL=m
> >
> > and the devices doesn't register on Raspberry Pi 3 B Plus:
> >
> > [   25.523337] vchiq: module is from the staging directory, the quality is unknown, you have been warned.
> > [   25.541647] bcm2835_vchiq 3f00b840.mailbox: Failed to register bcm2835_audio vchiq device
> > [   25.553692] bcm2835_vchiq 3f00b840.mailbox: Failed to register bcm2835-camera vchiq device
> 
> I was able to reproduce and it seems the issue here is the change 
> mentioned in the cover
> 
> - drop dma_set_mask_and_coherent
> 
> in V6.
> 
> (I usually test patches on RPi 4B with vcsm-cma and bcm2835-isp applied 
> so my branch has the DMA hunk included while I was testing V6)
> 
> Below is the hunk which should resolve the issue.
> 
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
> @@ -6,6 +6,7 @@
>    */
> 
>   #include <linux/device/bus.h>
> +#include <linux/dma-mapping.h>
>   #include <linux/slab.h>
>   #include <linux/string.h>
> 
> @@ -72,6 +73,12 @@ int vchiq_device_register(struct device *parent, 
> const char *name)
>          device->dev.type = &vchiq_device_type;
>          device->dev.release = vchiq_device_release;
> 
> +       ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32));
> +       if (ret < 0) {
> +               vchiq_device_release(&device->dev);
> +               return ret;
> +       }
> +
>          ret = device_register(&device->dev);
>          if (ret) {
>                  put_device(&device->dev);
> 
> It seems we need to include the dma_set_mask_and_coherent() even if 
> bcm2835-audio, bcm2835-camera device doesn't do DMA? I need to look into 
> why is that/
> 
>   Laurent, any thoughts on this please?

Nothing that immediately springs to my mind. Can you investigate ?
Umang Jain Jan. 23, 2023, 2:12 p.m. UTC | #3
Hi Stefan

On 1/23/23 5:16 PM, Stefan Wahren wrote:
> Hi Umang,
>
> Am 23.01.23 um 08:48 schrieb Umang Jain:
>> Hi Stefan,
>>
>> Thank for the testing.
>>
>> On 1/23/23 5:04 AM, Stefan Wahren wrote:
>>> Hi Umang,
>>>
>>> Am 20.01.23 um 21:10 schrieb Umang Jain:
>>>> This series just introduces five extra patches for dropping include
>>>> directives from Makefiles (suggested by Greg KH) and rebased.
>>>>
>>>> The main patch (6/6) removes platform device/driver abuse and moves
>>>> things to standard device/driver model using a custom_bus. Specific
>>>> details are elaborated in the commit message.
>>>>
>>>> The patch series is based on top of d514392f17fd (tag: next-20230120)
>>>> of linux-next.
>>>
>>> applied this series on top of linux-next and build it with 
>>> arm/multi_v7_defconfig plus the following:
>>>
>>> CONFIG_BCM_VIDEOCORE=y
>>> CONFIG_BCM2835_VCHIQ=m
>>> CONFIG_VCHIQ_CDEV=y
>>> CONFIG_SND_BCM2835=m
>>> CONFIG_VIDEO_BCM2835=m
>>> CONFIG_BCM2835_VCHIQ_MMAL=m
>>>
>>> and the devices doesn't register on Raspberry Pi 3 B Plus:
>>>
>>> [   25.523337] vchiq: module is from the staging directory, the 
>>> quality is unknown, you have been warned.
>>> [   25.541647] bcm2835_vchiq 3f00b840.mailbox: Failed to register 
>>> bcm2835_audio vchiq device
>>> [   25.553692] bcm2835_vchiq 3f00b840.mailbox: Failed to register 
>>> bcm2835-camera vchiq device
>>
>> I was able to reproduce and it seems the issue here is the change 
>> mentioned in the cover
>>
>> - drop dma_set_mask_and_coherent
>>
>> in V6.
>>
>> (I usually test patches on RPi 4B with vcsm-cma and bcm2835-isp 
>> applied so my branch has the DMA hunk included while I was testing V6)
>
> just to avoid misunderstandings, did you read 
> drivers/staging/vc04_services/interface/TESTING ?

Yes, but it's not geared towards the hardware I have (Rpi Model 4B)
>
> The Raspberry Pi 4 is currently not fully supported for VCHIQ in 
> mainline. From my limited understanding the DMA controller driver 
> needs 40 bit support, so VCHIQ on BCM2711 can use it. This part is 
> currently only available in the vendor tree. The reason why the driver 
> is unexpectedly probing is a historical issue in the devicetree :-(
>
> Raspberry Pi 4 support is not considered as necessary for moving out 
> of staging.
>

While I am looking into what's the reason behind the hunk, without which 
the devices cannot get registered... This is not related to the 40bit 
support for DMA. I remember that it worked without it during the 
bcm2835-isp development.

Are you implying that I should not use RPi Model 4B for testing ? 
Because that's the only RPi hardware I own.
>>
>> Below is the hunk which should resolve the issue.
>>
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
>> @@ -6,6 +6,7 @@
>>   */
>>
>>  #include <linux/device/bus.h>
>> +#include <linux/dma-mapping.h>
>>  #include <linux/slab.h>
>>  #include <linux/string.h>
>>
>> @@ -72,6 +73,12 @@ int vchiq_device_register(struct device *parent, 
>> const char *name)
>>         device->dev.type = &vchiq_device_type;
>>         device->dev.release = vchiq_device_release;
>>
>> +       ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32));
>> +       if (ret < 0) {
>> +               vchiq_device_release(&device->dev);
>> +               return ret;
>> +       }
>> +
>>         ret = device_register(&device->dev);
>>         if (ret) {
>>                 put_device(&device->dev);
>>
>> It seems we need to include the dma_set_mask_and_coherent() even if 
>> bcm2835-audio, bcm2835-camera device doesn't do DMA? I need to look 
>> into why is that/
>>
>>  Laurent, any thoughts on this please?
Stefan Wahren Jan. 23, 2023, 3:28 p.m. UTC | #4
Hi Umang,

Am 23.01.23 um 15:12 schrieb Umang Jain:
> Hi Stefan
>
> On 1/23/23 5:16 PM, Stefan Wahren wrote:
>> Hi Umang,
>>
>> Am 23.01.23 um 08:48 schrieb Umang Jain:
>>> Hi Stefan,
>>>
>>> Thank for the testing.
>>>
>>> On 1/23/23 5:04 AM, Stefan Wahren wrote:
>>>> Hi Umang,
>>>>
>>>> Am 20.01.23 um 21:10 schrieb Umang Jain:
>>>>> This series just introduces five extra patches for dropping include
>>>>> directives from Makefiles (suggested by Greg KH) and rebased.
>>>>>
>>>>> The main patch (6/6) removes platform device/driver abuse and moves
>>>>> things to standard device/driver model using a custom_bus. Specific
>>>>> details are elaborated in the commit message.
>>>>>
>>>>> The patch series is based on top of d514392f17fd (tag: next-20230120)
>>>>> of linux-next.
>>>>
>>>> applied this series on top of linux-next and build it with 
>>>> arm/multi_v7_defconfig plus the following:
>>>>
>>>> CONFIG_BCM_VIDEOCORE=y
>>>> CONFIG_BCM2835_VCHIQ=m
>>>> CONFIG_VCHIQ_CDEV=y
>>>> CONFIG_SND_BCM2835=m
>>>> CONFIG_VIDEO_BCM2835=m
>>>> CONFIG_BCM2835_VCHIQ_MMAL=m
>>>>
>>>> and the devices doesn't register on Raspberry Pi 3 B Plus:
>>>>
>>>> [   25.523337] vchiq: module is from the staging directory, the 
>>>> quality is unknown, you have been warned.
>>>> [   25.541647] bcm2835_vchiq 3f00b840.mailbox: Failed to register 
>>>> bcm2835_audio vchiq device
>>>> [   25.553692] bcm2835_vchiq 3f00b840.mailbox: Failed to register 
>>>> bcm2835-camera vchiq device
>>>
>>> I was able to reproduce and it seems the issue here is the change 
>>> mentioned in the cover
>>>
>>> - drop dma_set_mask_and_coherent
>>>
>>> in V6.
>>>
>>> (I usually test patches on RPi 4B with vcsm-cma and bcm2835-isp 
>>> applied so my branch has the DMA hunk included while I was testing V6)
>>
>> just to avoid misunderstandings, did you read 
>> drivers/staging/vc04_services/interface/TESTING ?
>
> Yes, but it's not geared towards the hardware I have (Rpi Model 4B)
>>
>> The Raspberry Pi 4 is currently not fully supported for VCHIQ in 
>> mainline. From my limited understanding the DMA controller driver 
>> needs 40 bit support, so VCHIQ on BCM2711 can use it. This part is 
>> currently only available in the vendor tree. The reason why the 
>> driver is unexpectedly probing is a historical issue in the 
>> devicetree :-(
>>
>> Raspberry Pi 4 support is not considered as necessary for moving out 
>> of staging.
>>
>
> While I am looking into what's the reason behind the hunk, without 
> which the devices cannot get registered... This is not related to the 
> 40bit support for DMA. I remember that it worked without it during the 
> bcm2835-isp development.
>
> Are you implying that I should not use RPi Model 4B for testing ? 
> Because that's the only RPi hardware I own.

No, i didn't want to imply this. It's just a warning that's currently 
not the best platform for testing. But i also understand that nowadays 
users have a BCM2711 based Raspberry Pi.

This leads to another question which device tree do you use for testing, 
since the mainline one doesn't have the right compatible for BCM2711?

>>>
>>> Below is the hunk which should resolve the issue.
>>>
>>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
>>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
>>> @@ -6,6 +6,7 @@
>>>   */
>>>
>>>  #include <linux/device/bus.h>
>>> +#include <linux/dma-mapping.h>
>>>  #include <linux/slab.h>
>>>  #include <linux/string.h>
>>>
>>> @@ -72,6 +73,12 @@ int vchiq_device_register(struct device *parent, 
>>> const char *name)
>>>         device->dev.type = &vchiq_device_type;
>>>         device->dev.release = vchiq_device_release;
>>>
>>> +       ret = dma_set_mask_and_coherent(&device->dev, 
>>> DMA_BIT_MASK(32));
>>> +       if (ret < 0) {
>>> +               vchiq_device_release(&device->dev);
>>> +               return ret;
>>> +       }
>>> +
>>>         ret = device_register(&device->dev);
>>>         if (ret) {
>>>                 put_device(&device->dev);
>>>
>>> It seems we need to include the dma_set_mask_and_coherent() even if 
>>> bcm2835-audio, bcm2835-camera device doesn't do DMA? I need to look 
>>> into why is that/
>>>
>>>  Laurent, any thoughts on this please?
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Laurent Pinchart Jan. 24, 2023, 8:26 a.m. UTC | #5
Hi Umang,

On Tue, Jan 24, 2023 at 11:09:31AM +0530, Umang Jain wrote:
> On 1/23/23 10:58 PM, Stefan Wahren wrote:
> > Am 23.01.23 um 08:48 schrieb Umang Jain:
> >> On 1/23/23 5:04 AM, Stefan Wahren wrote:
> >>> Am 20.01.23 um 21:10 schrieb Umang Jain:
> >>>> This series just introduces five extra patches for dropping include
> >>>> directives from Makefiles (suggested by Greg KH) and rebased.
> >>>>
> >>>> The main patch (6/6) removes platform device/driver abuse and moves
> >>>> things to standard device/driver model using a custom_bus. Specific
> >>>> details are elaborated in the commit message.
> >>>>
> >>>> The patch series is based on top of d514392f17fd (tag: next-20230120)
> >>>> of linux-next.
> >>>
> >>> applied this series on top of linux-next and build it with 
> >>> arm/multi_v7_defconfig plus the following:
> >>>
> >>> CONFIG_BCM_VIDEOCORE=y
> >>> CONFIG_BCM2835_VCHIQ=m
> >>> CONFIG_VCHIQ_CDEV=y
> >>> CONFIG_SND_BCM2835=m
> >>> CONFIG_VIDEO_BCM2835=m
> >>> CONFIG_BCM2835_VCHIQ_MMAL=m
> >>>
> >>> and the devices doesn't register on Raspberry Pi 3 B Plus:
> >>>
> >>> [   25.523337] vchiq: module is from the staging directory, the 
> >>> quality is unknown, you have been warned.
> >>> [   25.541647] bcm2835_vchiq 3f00b840.mailbox: Failed to register 
> >>> bcm2835_audio vchiq device
> >>> [   25.553692] bcm2835_vchiq 3f00b840.mailbox: Failed to register 
> >>> bcm2835-camera vchiq device
> >>
> >> I was able to reproduce and it seems the issue here is the change 
> >> mentioned in the cover
> >>
> >> - drop dma_set_mask_and_coherent
> >>
> >> in V6.
> >>
> >> (I usually test patches on RPi 4B with vcsm-cma and bcm2835-isp 
> >> applied so my branch has the DMA hunk included while I was testing V6)
> >>
> >> Below is the hunk which should resolve the issue.
> >>
> >> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
> >> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
> >> @@ -6,6 +6,7 @@
> >>   */
> >>
> >>  #include <linux/device/bus.h>
> >> +#include <linux/dma-mapping.h>
> >>  #include <linux/slab.h>
> >>  #include <linux/string.h>
> >>
> >> @@ -72,6 +73,12 @@ int vchiq_device_register(struct device *parent, 
> >> const char *name)
> >>         device->dev.type = &vchiq_device_type;
> >>         device->dev.release = vchiq_device_release;
> >>
> >> +       ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32));
> >> +       if (ret < 0) {
> >> +               vchiq_device_release(&device->dev);
> >> +               return ret;
> >> +       }
> >> +
> >>         ret = device_register(&device->dev);
> >>         if (ret) {
> >>                 put_device(&device->dev);
> >
> > Yes, this patch fixes the errors above. But i noticed that the series 
> > also break autoprobing of bcm2835-audio and bcm2835-camera.
> 
> For the diff concerned, I am still looking into why is this needed.
> 
> Regarding the autoprobing, I have noticed that as well. It seems the 
> probing is automatic for platform driver/devices and we are moving away 
> from the platform driver/devices. So, this is expected I suppose?
> 
> Reading from Documentation/driver-api/driver-model/platform.rst
> 
> """
> Driver binding is performed automatically by the driver core, invoking 
> driver probe() after finding a match between device and driver. If the 
> probe() succeeds, the driver and device are bound as usual
> """
> 
> Should we retain this behavior ?

Why shouldn't we ? :-)

> >> It seems we need to include the dma_set_mask_and_coherent() even if 
> >> bcm2835-audio, bcm2835-camera device doesn't do DMA? I need to look 
> >> into why is that/
> >>
> >>  Laurent, any thoughts on this please?
Stefan Wahren Jan. 24, 2023, 8:41 a.m. UTC | #6
Hi Phil,

Am 23.01.23 um 09:58 schrieb Laurent Pinchart:
> On Mon, Jan 23, 2023 at 01:18:30PM +0530, Umang Jain wrote:
>> Hi Stefan,
>>
>> Thank for the testing.
>>
>> On 1/23/23 5:04 AM, Stefan Wahren wrote:
>>> Hi Umang,
>>>
>>> Am 20.01.23 um 21:10 schrieb Umang Jain:
>>>> This series just introduces five extra patches for dropping include
>>>> directives from Makefiles (suggested by Greg KH) and rebased.
>>>>
>>>> The main patch (6/6) removes platform device/driver abuse and moves
>>>> things to standard device/driver model using a custom_bus. Specific
>>>> details are elaborated in the commit message.
>>>>
>>>> The patch series is based on top of d514392f17fd (tag: next-20230120)
>>>> of linux-next.
>>> applied this series on top of linux-next and build it with
>>> arm/multi_v7_defconfig plus the following:
>>>
>>> CONFIG_BCM_VIDEOCORE=y
>>> CONFIG_BCM2835_VCHIQ=m
>>> CONFIG_VCHIQ_CDEV=y
>>> CONFIG_SND_BCM2835=m
>>> CONFIG_VIDEO_BCM2835=m
>>> CONFIG_BCM2835_VCHIQ_MMAL=m
>>>
>>> and the devices doesn't register on Raspberry Pi 3 B Plus:
>>>
>>> [   25.523337] vchiq: module is from the staging directory, the quality is unknown, you have been warned.
>>> [   25.541647] bcm2835_vchiq 3f00b840.mailbox: Failed to register bcm2835_audio vchiq device
>>> [   25.553692] bcm2835_vchiq 3f00b840.mailbox: Failed to register bcm2835-camera vchiq device
>> I was able to reproduce and it seems the issue here is the change
>> mentioned in the cover
>>
>> - drop dma_set_mask_and_coherent
>>
>> in V6.
>>
>> (I usually test patches on RPi 4B with vcsm-cma and bcm2835-isp applied
>> so my branch has the DMA hunk included while I was testing V6)
>>
>> Below is the hunk which should resolve the issue.
>>
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
>> @@ -6,6 +6,7 @@
>>     */
>>
>>    #include <linux/device/bus.h>
>> +#include <linux/dma-mapping.h>
>>    #include <linux/slab.h>
>>    #include <linux/string.h>
>>
>> @@ -72,6 +73,12 @@ int vchiq_device_register(struct device *parent,
>> const char *name)
>>           device->dev.type = &vchiq_device_type;
>>           device->dev.release = vchiq_device_release;
>>
>> +       ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32));
>> +       if (ret < 0) {
>> +               vchiq_device_release(&device->dev);
>> +               return ret;
>> +       }
>> +
>>           ret = device_register(&device->dev);
>>           if (ret) {
>>                   put_device(&device->dev);
>>
>> It seems we need to include the dma_set_mask_and_coherent() even if
>> bcm2835-audio, bcm2835-camera device doesn't do DMA? I need to look into
>> why is that/

Do you have an answer for this?

Thanks

>>
>>    Laurent, any thoughts on this please?
> Nothing that immediately springs to my mind. Can you investigate ?
>
Phil Elwell Jan. 24, 2023, 7:47 p.m. UTC | #7
On 24/01/2023 8:41, Stefan Wahren wrote:
> Hi Phil,
> 
> Am 23.01.23 um 09:58 schrieb Laurent Pinchart:
>> On Mon, Jan 23, 2023 at 01:18:30PM +0530, Umang Jain wrote:
>>> Hi Stefan,
>>>
>>> Thank for the testing.
>>>
>>> On 1/23/23 5:04 AM, Stefan Wahren wrote:
>>>> Hi Umang,
>>>>
>>>> Am 20.01.23 um 21:10 schrieb Umang Jain:
>>>>> This series just introduces five extra patches for dropping include
>>>>> directives from Makefiles (suggested by Greg KH) and rebased.
>>>>>
>>>>> The main patch (6/6) removes platform device/driver abuse and moves
>>>>> things to standard device/driver model using a custom_bus. Specific
>>>>> details are elaborated in the commit message.
>>>>>
>>>>> The patch series is based on top of d514392f17fd (tag: next-20230120)
>>>>> of linux-next.
>>>> applied this series on top of linux-next and build it with
>>>> arm/multi_v7_defconfig plus the following:
>>>>
>>>> CONFIG_BCM_VIDEOCORE=y
>>>> CONFIG_BCM2835_VCHIQ=m
>>>> CONFIG_VCHIQ_CDEV=y
>>>> CONFIG_SND_BCM2835=m
>>>> CONFIG_VIDEO_BCM2835=m
>>>> CONFIG_BCM2835_VCHIQ_MMAL=m
>>>>
>>>> and the devices doesn't register on Raspberry Pi 3 B Plus:
>>>>
>>>> [   25.523337] vchiq: module is from the staging directory, the quality is unknown, you have been warned.
>>>> [   25.541647] bcm2835_vchiq 3f00b840.mailbox: Failed to register bcm2835_audio vchiq device
>>>> [   25.553692] bcm2835_vchiq 3f00b840.mailbox: Failed to register bcm2835-camera vchiq device
>>> I was able to reproduce and it seems the issue here is the change
>>> mentioned in the cover
>>>
>>> - drop dma_set_mask_and_coherent
>>>
>>> in V6.
>>>
>>> (I usually test patches on RPi 4B with vcsm-cma and bcm2835-isp applied
>>> so my branch has the DMA hunk included while I was testing V6)
>>>
>>> Below is the hunk which should resolve the issue.
>>>
>>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
>>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_device.c
>>> @@ -6,6 +6,7 @@
>>>     */
>>>
>>>    #include <linux/device/bus.h>
>>> +#include <linux/dma-mapping.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/string.h>
>>>
>>> @@ -72,6 +73,12 @@ int vchiq_device_register(struct device *parent,
>>> const char *name)
>>>           device->dev.type = &vchiq_device_type;
>>>           device->dev.release = vchiq_device_release;
>>>
>>> +       ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32));
>>> +       if (ret < 0) {
>>> +               vchiq_device_release(&device->dev);
>>> +               return ret;
>>> +       }
>>> +
>>>           ret = device_register(&device->dev);
>>>           if (ret) {
>>>                   put_device(&device->dev);
>>>
>>> It seems we need to include the dma_set_mask_and_coherent() even if
>>> bcm2835-audio, bcm2835-camera device doesn't do DMA? I need to look into
>>> why is that/
> 
> Do you have an answer for this?

That's because vchiq does use DMA for bulk transfers, it's just that the DMA hardware
is driven from the VPU side. And even though the VPU can only address 1GB, it uses the
upper address bits to determine cacheability of accesses, hence the need for 32-bit
DMA addresses.

Phil