linux-kernel - Re: [PATCH v4 2/6] i3c: master: svc: add hot join support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231130111636.34432d30@xps-13>
Date:   Thu, 30 Nov 2023 11:16:36 +0100
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Frank Li <Frank.Li@....com>
Cc:     alexandre.belloni@...tlin.com, conor.culhane@...vaco.com,
        imx@...ts.linux.dev, joe@...ches.com,
        linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org,
        zbigniew.lukwinski@...ux.intel.com
Subject: Re: [PATCH v4 2/6] i3c: master: svc: add hot join support

Hi Frank,

Frank.Li@....com wrote on Wed, 29 Nov 2023 17:12:21 -0500:

> Add hot join support for svc master controller. Enable hot join defaultly.

							by default

> User can use sys entry to disable hot join.

Should we do the opposite? Disable hot-join by default and allow
enabling it?

> 
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
>  drivers/i3c/master/svc-i3c-master.c | 58 +++++++++++++++++++++++++++--
>  1 file changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 6b6bdd163af4f..880c6ae76c013 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -128,6 +128,9 @@
>  /* This parameter depends on the implementation and may be tuned */
>  #define SVC_I3C_FIFO_SIZE 16
>  
> +#define SVC_I3C_EVENT_IBI	BIT(0)
> +#define SVC_I3C_EVENT_HOTJOIN	BIT(1)
> +
>  struct svc_i3c_cmd {
>  	u8 addr;
>  	bool rnw;
> @@ -176,6 +179,7 @@ struct svc_i3c_regs_save {
>   * @ibi.tbq_slot: To be queued IBI slot
>   * @ibi.lock: IBI lock
>   * @lock: Transfer lock, protect between IBI work thread and callbacks from master
> + * @enabled_events: Bit masks for enable events (IBI, HotJoin).
>   */
>  struct svc_i3c_master {
>  	struct i3c_master_controller base;
> @@ -205,6 +209,7 @@ struct svc_i3c_master {
>  		spinlock_t lock;
>  	} ibi;
>  	struct mutex lock;
> +	int enabled_events;
>  };
>  
>  /**
> @@ -428,13 +433,16 @@ static void svc_i3c_master_ibi_work(struct work_struct *work)
>  	switch (ibitype) {
>  	case SVC_I3C_MSTATUS_IBITYPE_IBI:
>  		dev = svc_i3c_master_dev_from_addr(master, ibiaddr);
> -		if (!dev)
> +		if (!dev || !(master->enabled_events & SVC_I3C_EVENT_IBI))

If we are going to add new events like that maybe we should have a
helper. So we can then extend the helper with the list of enabled
events?

>  			svc_i3c_master_nack_ibi(master);
>  		else
>  			svc_i3c_master_handle_ibi(master, dev);
>  		break;
>  	case SVC_I3C_MSTATUS_IBITYPE_HOT_JOIN:
> -		svc_i3c_master_ack_ibi(master, false);
> +		if (master->enabled_events & SVC_I3C_EVENT_HOTJOIN)

And you could reuse the helper here

> +			svc_i3c_master_ack_ibi(master, false);
> +		else
> +			svc_i3c_master_nack_ibi(master);
>  		break;
>  	case SVC_I3C_MSTATUS_IBITYPE_MASTER_REQUEST:
>  		svc_i3c_master_nack_ibi(master);
> @@ -471,7 +479,9 @@ static void svc_i3c_master_ibi_work(struct work_struct *work)
>  		svc_i3c_master_emit_stop(master);
>  		break;
>  	case SVC_I3C_MSTATUS_IBITYPE_HOT_JOIN:
> -		queue_work(master->base.wq, &master->hj_work);
> +		svc_i3c_master_emit_stop(master);
> +		if (master->enabled_events & SVC_I3C_EVENT_HOTJOIN)

and here

> +			queue_work(master->base.wq, &master->hj_work);
>  		break;

Thanks,
Miquèl