On Fri, Jan 20, 2023 at 03:52:22AM +0200, Laurent Pinchart wrote:
> Hi Umang,
>
> Thank you for the patch.
>
> On Thu, Jan 19, 2023 at 05:25:03PM +0530, Umang Jain wrote:
> > The devices that the vchiq interface registers(bcm2835-audio,
>
> Missing space before '('.
>
> > bcm2835-camera) are implemented and exposed by the VC04 firmware.
> > The device tree describes the VC04 itself with the resources
> > required to communicate with it through a mailbox interface. However,
> > the vchiq interface registers these devices as platform devices. This
> > also means the specific drivers for these devices are also getting
>
> Drop one of the two "also".
>
> > registered as platform drivers. This is not correct and a blatant
> > abuse of platform device/driver.
> >
> > Replace the platform device/driver model with a standard device driver
> > model. A custom bus_type, vchiq_bus_type, is created in the vchiq
> > interface which matches the devices to their specific device drivers
> > thereby, establishing driver binding. A struct vchiq_device wraps the
> > struct device for each device being registered on the bus by the vchiq
> > interface.
> >
> > Each device registered will expose a 'name' read-only device attribute
> > in sysfs (/sys/bus/vchiq-bus/devices). New devices and drivers can be
> > added by registering on vchiq_bus_type and adding a corresponding
> > device name entry in the static list of devices, vchiq_devices. There
> > is currently no way to enumerate the VCHIQ devices that are available
> > from the firmware.
>
> Greg, I don't know if you've followed the conversation in earlier mail
> threads, so I'll try to summarize it here.
>
> There are two layers involved: the VCHIQ layer, which has two clients
> (audio and MMAL), and the MMAL layer, which has multiple clients
> (camera, codec, ISP). The reason for this is that audio and mmal are
> separate hardware, while camera, codec and ISP share some hardware
> blocks.
>
> The VCHIQ layer provides a mailbox API to its clients to communicate
> with the firmware, and the MMAL layer provides another API implemented
> on top of the VCHIQ layer. Neither APIs offer a way to discover devices
> dynamically (that's not a feature implemented by the firmware). We've
> decided that implementing two buses would be overkill, so Umang went for
> a single vchiq_bus_type. The only value it provides is to stop abusing
> platform_device. That's pretty much it.
>
> Given the above explanation, do you still think the additional
> complexity introduced by the vchiq bus type is worth it (it more or less
> duplicates a small subset of the platform bus type implementation), and
> are you fine with a single bus type, even if it doesn't exactly match
> the firmware layers ?
Yes, this is the correct way forward. I didn't review the changes yet,
but I see you just gave a good first pass, so I'll wait for the next
revision.
thanks,
greg k-h
