Re: [PATCH 08/11] staging: vchiq_core: Add comments to mutex/spinlocks

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Stefan and Phil,

Thank you for the patch.

On Tue, Jun 04, 2024 at 07:29:01PM +0200, Stefan Wahren wrote:
> From: Phil Elwell <phil@raspberrypi.com>
> 
> checkpatch.pl complains about missing comments at
> mutex/spinlock definitions. So add them accordingly.

Less warnings is good, but we should address the problem they outline,
not just silence them for the sake of it. It would be better, for each
lock, to explicitly list the data fields the lock protects.

> Link: https://github.com/raspberrypi/linux/pull/6139/
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> ---
>  .../vc04_services/interface/vchiq_arm/vchiq_core.h     | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> index 3c7a6838ddba..3abcd6910f25 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> @@ -196,6 +196,8 @@ struct vchiq_service {
> 
>  	struct completion remove_event;
>  	struct completion bulk_remove_event;
> +
> +	/* Serialise access to the bulk transfer queues */
>  	struct mutex bulk_mutex;
> 
>  	struct service_stats_struct {
> @@ -312,7 +314,7 @@ struct vchiq_state {
>  	/* Event indicating connect message received */
>  	struct completion connect;
> 
> -	/* Mutex protecting services */
> +	/* Mutex protecting service creation */
>  	struct mutex mutex;
>  	struct vchiq_instance **instance;
> 
> @@ -341,16 +343,22 @@ struct vchiq_state {
>  	char *rx_data;
>  	struct vchiq_slot_info *rx_info;
> 
> +	/* Serialise access to the main message slots */
>  	struct mutex slot_mutex;
> 
> +	/* Serialise slot refcount updates */
>  	struct mutex recycle_mutex;
> 
> +	/* Serialise access to the single synchronous message slot */
>  	struct mutex sync_mutex;
> 
> +	/* Serialise access to the message queues to userspace */
>  	spinlock_t msg_queue_spinlock;
> 
> +	/* Serialise completion of blocking transfers */
>  	spinlock_t bulk_waiter_spinlock;
> 
> +	/* Serialise updates to slot quota data */
>  	spinlock_t quota_spinlock;
> 
>  	/*

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel