Re: [PATCH v3] staging: vc04_services: vchiq: Register devices with a custom bus_type

Greg Kroah-Hartman
2023-01-17T19:09:35Z

On Wed, Jan 11, 2023 at 02:30:29PM +0530, Umang Jain wrote:
> The devices that the vchiq interface registers(bcm2835-audio,
> 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
> 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.
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> 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/
>
> - Developed on top of:
> [PATCH v3 0/7] staging: vc04_services: Remove custom return values
>
> ---
> .../vc04_services/bcm2835-audio/Makefile | 5 +-
> .../vc04_services/bcm2835-audio/bcm2835.c | 19 ++-
> .../vc04_services/bcm2835-camera/Makefile | 2 +
> .../bcm2835-camera/bcm2835-camera.c | 17 ++-
> .../interface/vchiq_arm/vchiq_arm.c | 121 +++++++++++++++---
> .../interface/vchiq_arm/vchiq_arm.h | 1 +
> 6 files changed, 123 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/Makefile b/drivers/staging/vc04_services/bcm2835-audio/Makefile
> index d59fe4dde615..4801467be58a 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/Makefile
> +++ b/drivers/staging/vc04_services/bcm2835-audio/Makefile
> @@ -2,4 +2,7 @@
> obj-$(CONFIG_SND_BCM2835) += snd-bcm2835.o
> snd-bcm2835-objs := bcm2835.o bcm2835-ctl.o bcm2835-pcm.o bcm2835-vchiq.o
>
> -ccflags-y += -I $(srctree)/$(src)/../include -D__VCCOREVER__=0x04000000
> +ccflags-y += \
> + -I $(srctree)/$(src)/../include \
> + -I $(srctree)/$(src)/../interface/vchiq_arm \

Please never add new -I entries, as that can break the build when you
only want to build a subdirectory. Just use the "../" in the .c files
instead.

While you are not responsible for the original one here, please never
add a new one.

And, you can work to get rid of the current addition as well, that
should be done sooner rather than later anyway.

thanks,

greg k-h