Re: [PATCH v10 1/5] staging: vc04_services: vchiq_arm: Add new bus type and device type

[Umang Jain <umang.jain@ideasonboard.com>]

Hi Robin

On 9/13/23 5:38 PM, Robin Murphy wrote:
> On 2023-09-12 06:50, Umang Jain wrote:
> [...]
>>>> +struct vchiq_device *
>>>> +vchiq_device_register(struct device *parent, const char *name)
>>>> +{
>>>> +    struct vchiq_device *device;
>>>> +    int ret;
>>>> +
>>>> +    device = kzalloc(sizeof(*device), GFP_KERNEL);
>>>> +    if (!device) {
>>>> +        dev_err(parent, "Cannot register %s: Insufficient memory\n",
>>>> +            name);
>>>> +        return NULL;
>>>> +    }
>>>> +
>>>> +    device->dev.init_name = name;
>>>> +    device->dev.parent = parent;
>>>> +    device->dev.bus = &vchiq_bus_type;
>>>> +    device->dev.release = vchiq_device_release;
>>>> +
>>>> +    of_dma_configure(&device->dev, parent->of_node, true);
>>>> +    ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32));
>>>> +    if (ret) {
>>>> +        dev_err(&device->dev, "32-bit DMA enable failed\n");
>>>> +        return NULL;
>>>> +    }
>>>
>>> Unfortunately the call of of_dma_configure() generates warnings likes
>>> this (Raspberry Pi 3A+ with multi_v7_defconfig + VCHIQ):
>>>
>>> [    9.206802] vchiq-bus bcm2835-audio: DMA mask not set
>>> [    9.206892] vchiq-bus bcm2835-camera: DMA mask not set
>>
>> huh, really weird, as on my RPi-3-b I get these set correctly and I 
>> don't any such warning.
>
> Can you point to the code above where device->dev.dma_mask gets 
> initialised between the initial kzalloc() and the call to 
> of_dma_configure()? ;)
>
> BTW, bus code shouldn't be calling dma_set_mask_and_coherent() on 
> behalf of its children, that is for the individual drivers to do, if 
> and when they intend to actually use DMA. Removing that here will save 
> you needing to fix the memory leak as well...

Thanks for this suggestion. I have now set the dma_mask within the child 
itself!
>
> Thanks,
> Robin.