[PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

System Controller Management Interface(SCMI) firmwares might have
different designs by SCMI firmware developers. So the maximum receive
channel timeout value might also varies in the various designs.

So introduce property mbox-rx-timeout-ms to let each platform could
set its own timeout value in device tree.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V2:
 Drop defaults, update description.

 Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index ebf384e76df1..dcac0b36c76f 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -121,6 +121,12 @@ properties:
       atomic mode of operation, even if requested.
     default: 0
 
+  max-rx-timeout-ms:
+    description:
+      An optional time value, expressed in milliseconds, representing the
+      mailbox maximum timeout value for receive channel. The value should
+      be a non-zero value if set.
+
   arm,smc-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description:
-- 
2.37.1

[PATCH V2 2/2] firmware: arm_scmi: set mailbox timeout value from device tree

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Each platform might have its own maximum mailbox receive channel timeout
value, so get property max-rx-timeout-ms from device tree and use it. If
the property does not exist, use mailbox 'scmi_desc' fixed value 30ms as
before.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V2:
 Fix build error for raw mode.

 drivers/firmware/arm_scmi/driver.c   | 19 ++++++++++++++-----
 drivers/firmware/arm_scmi/raw_mode.c | 13 +++++++++----
 drivers/firmware/arm_scmi/raw_mode.h |  3 ++-
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 6b6957f4743f..1aa613d4cb43 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -162,6 +162,7 @@ struct scmi_debug_info {
  * @devreq_mtx: A mutex to serialize device creation for this SCMI instance
  * @dbg: A pointer to debugfs related data (if any)
  * @raw: An opaque reference handle used by SCMI Raw mode.
+ * @max_rx_timeout_ms: the maximum receive channel timeout value
  */
 struct scmi_info {
 	int id;
@@ -188,6 +189,7 @@ struct scmi_info {
 	struct mutex devreq_mtx;
 	struct scmi_debug_info *dbg;
 	void *raw;
+	unsigned int max_rx_timeout_ms;
 };
 
 #define handle_to_scmi_info(h)	container_of(h, struct scmi_info, handle)
@@ -1302,11 +1304,11 @@ static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo,
 
 	trace_scmi_xfer_response_wait(xfer->transfer_id, xfer->hdr.id,
 				      xfer->hdr.protocol_id, xfer->hdr.seq,
-				      info->desc->max_rx_timeout_ms,
+				      info->max_rx_timeout_ms,
 				      xfer->hdr.poll_completion);
 
 	return scmi_wait_for_reply(dev, info->desc, cinfo, xfer,
-				   info->desc->max_rx_timeout_ms);
+				   info->max_rx_timeout_ms);
 }
 
 /**
@@ -2614,7 +2616,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
 	if (!cinfo)
 		return -ENOMEM;
 
-	cinfo->rx_timeout_ms = info->desc->max_rx_timeout_ms;
+	cinfo->rx_timeout_ms = info->max_rx_timeout_ms;
 
 	/* Create a unique name for this transport device */
 	snprintf(name, 32, "__scmi_transport_device_%s_%02X",
@@ -2888,7 +2890,7 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info)
 	debugfs_create_bool("is_atomic", 0400, trans, &dbg->is_atomic);
 
 	debugfs_create_u32("max_rx_timeout_ms", 0400, trans,
-			   (u32 *)&info->desc->max_rx_timeout_ms);
+			   (u32 *)&info->max_rx_timeout_ms);
 
 	debugfs_create_u32("max_msg_size", 0400, trans,
 			   (u32 *)&info->desc->max_msg_size);
@@ -2940,7 +2942,8 @@ static int scmi_debugfs_raw_mode_setup(struct scmi_info *info)
 
 	info->raw = scmi_raw_mode_init(&info->handle, info->dbg->top_dentry,
 				       info->id, channels, num_chans,
-				       info->desc, info->tx_minfo.max_msg);
+				       info->desc, info->tx_minfo.max_msg,
+				       info->max_rx_timeout_ms);
 	if (IS_ERR(info->raw)) {
 		dev_err(info->dev, "Failed to initialize SCMI RAW Mode !\n");
 		ret = PTR_ERR(info->raw);
@@ -2953,6 +2956,7 @@ static int scmi_debugfs_raw_mode_setup(struct scmi_info *info)
 static int scmi_probe(struct platform_device *pdev)
 {
 	int ret;
+	u32 timeout;
 	char *err_str = "probe failure\n";
 	struct scmi_handle *handle;
 	const struct scmi_desc *desc;
@@ -3002,6 +3006,11 @@ static int scmi_probe(struct platform_device *pdev)
 			 info->atomic_threshold);
 	handle->is_transport_atomic = scmi_is_transport_atomic;
 
+	if (!of_property_read_u32(np, "max-rx-timeout-ms", &timeout))
+		info->max_rx_timeout_ms = timeout;
+	else
+		info->max_rx_timeout_ms = info->desc->max_rx_timeout_ms;
+
 	if (desc->ops->link_supplier) {
 		ret = desc->ops->link_supplier(dev);
 		if (ret) {
diff --git a/drivers/firmware/arm_scmi/raw_mode.c b/drivers/firmware/arm_scmi/raw_mode.c
index 130d13e9cd6b..06a764d106f8 100644
--- a/drivers/firmware/arm_scmi/raw_mode.c
+++ b/drivers/firmware/arm_scmi/raw_mode.c
@@ -165,6 +165,7 @@ struct scmi_raw_queue {
  * @wait_wq: A workqueue reference to the created workqueue
  * @dentry: Top debugfs root dentry for SCMI Raw
  * @gid: A group ID used for devres accounting
+ * @max_rx_timeout_ms: Max receive channel timeout value
  *
  * Note that this descriptor is passed back to the core after SCMI Raw is
  * initialized as an opaque handle to use by subsequent SCMI Raw call hooks.
@@ -187,6 +188,7 @@ struct scmi_raw_mode_info {
 	struct workqueue_struct	*wait_wq;
 	struct dentry *dentry;
 	void *gid;
+	u32 max_rx_timeout_ms;
 };
 
 /**
@@ -379,7 +381,7 @@ static void scmi_xfer_raw_waiter_enqueue(struct scmi_raw_mode_info *raw,
 	trace_scmi_xfer_response_wait(rw->xfer->transfer_id, rw->xfer->hdr.id,
 				      rw->xfer->hdr.protocol_id,
 				      rw->xfer->hdr.seq,
-				      raw->desc->max_rx_timeout_ms,
+				      raw->max_rx_timeout_ms,
 				      rw->xfer->hdr.poll_completion);
 
 	mutex_lock(&raw->active_mtx);
@@ -437,7 +439,7 @@ static void scmi_xfer_raw_worker(struct work_struct *work)
 
 	raw = container_of(work, struct scmi_raw_mode_info, waiters_work);
 	dev = raw->handle->dev;
-	max_tmo = msecs_to_jiffies(raw->desc->max_rx_timeout_ms);
+	max_tmo = msecs_to_jiffies(raw->max_rx_timeout_ms);
 
 	do {
 		int ret = 0;
@@ -574,7 +576,7 @@ static int scmi_xfer_raw_get_init(struct scmi_raw_mode_info *raw, void *buf,
 			dev_dbg(dev,
 				"...retrying[%d] inflight registration\n",
 				retry);
-			msleep(raw->desc->max_rx_timeout_ms /
+			msleep(raw->max_rx_timeout_ms /
 			       SCMI_XFER_RAW_MAX_RETRIES);
 		}
 	} while (ret && --retry);
@@ -1162,6 +1164,7 @@ static int scmi_raw_mode_setup(struct scmi_raw_mode_info *raw,
  * @num_chans: The number of entries in @channels
  * @desc: Reference to the transport operations
  * @tx_max_msg: Max number of in-flight messages allowed by the transport
+ * @max_rx_timeout_ms: Max receive channel timeout value
  *
  * This function prepare the SCMI Raw stack and creates the debugfs API.
  *
@@ -1170,7 +1173,8 @@ static int scmi_raw_mode_setup(struct scmi_raw_mode_info *raw,
 void *scmi_raw_mode_init(const struct scmi_handle *handle,
 			 struct dentry *top_dentry, int instance_id,
 			 u8 *channels, int num_chans,
-			 const struct scmi_desc *desc, int tx_max_msg)
+			 const struct scmi_desc *desc, int tx_max_msg,
+			 u32 max_rx_timeout_ms)
 {
 	int ret;
 	struct scmi_raw_mode_info *raw;
@@ -1188,6 +1192,7 @@ void *scmi_raw_mode_init(const struct scmi_handle *handle,
 	raw->desc = desc;
 	raw->tx_max_msg = tx_max_msg;
 	raw->id = instance_id;
+	raw->max_rx_timeout_ms = max_rx_timeout_ms;
 
 	ret = scmi_raw_mode_setup(raw, channels, num_chans);
 	if (ret) {
diff --git a/drivers/firmware/arm_scmi/raw_mode.h b/drivers/firmware/arm_scmi/raw_mode.h
index 8af756a83fd1..25d4a46261e7 100644
--- a/drivers/firmware/arm_scmi/raw_mode.h
+++ b/drivers/firmware/arm_scmi/raw_mode.h
@@ -20,7 +20,8 @@ enum {
 void *scmi_raw_mode_init(const struct scmi_handle *handle,
 			 struct dentry *top_dentry, int instance_id,
 			 u8 *channels, int num_chans,
-			 const struct scmi_desc *desc, int tx_max_msg);
+			 const struct scmi_desc *desc, int tx_max_msg,
+			 u32 max_rx_timeout_ms);
 void scmi_raw_mode_cleanup(void *raw);
 
 void scmi_raw_message_report(void *raw, struct scmi_xfer *xfer,
-- 
2.37.1

Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Sudeep Holla]

On Wed, Jul 03, 2024 at 11:17:14AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> System Controller Management Interface(SCMI) firmwares might have
> different designs by SCMI firmware developers. So the maximum receive
> channel timeout value might also varies in the various designs.
> 
> So introduce property mbox-rx-timeout-ms to let each platform could
> set its own timeout value in device tree.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> V2:
>  Drop defaults, update description.
> 
>  Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index ebf384e76df1..dcac0b36c76f 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -121,6 +121,12 @@ properties:
>        atomic mode of operation, even if requested.
>      default: 0
>  
> +  max-rx-timeout-ms:
> +    description:
> +      An optional time value, expressed in milliseconds, representing the
> +      mailbox maximum timeout value for receive channel. The value should
> +      be a non-zero value if set.
> +

IIRC, you had the min and max constraint in the earlier response. You
need to have rushed and posted another version before I could respond
with my preference.

So there is no rush, these are v6.12 material. Take time for respining
and give some time for the review.

-- 
Regards,
Sudeep

RE: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Peng Fan]

> Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce
> property mbox-rx-timeout-ms
> 
> On Wed, Jul 03, 2024 at 11:17:14AM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > System Controller Management Interface(SCMI) firmwares might
> have
> > different designs by SCMI firmware developers. So the maximum
> receive
> > channel timeout value might also varies in the various designs.
> >
> > So introduce property mbox-rx-timeout-ms to let each platform could
> > set its own timeout value in device tree.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > V2:
> >  Drop defaults, update description.
> >
> >  Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6
> ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git
> a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > index ebf384e76df1..dcac0b36c76f 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > @@ -121,6 +121,12 @@ properties:
> >        atomic mode of operation, even if requested.
> >      default: 0
> >
> > +  max-rx-timeout-ms:
> > +    description:
> > +      An optional time value, expressed in milliseconds, representing
> the
> > +      mailbox maximum timeout value for receive channel. The value
> should
> > +      be a non-zero value if set.
> > +
> 
> IIRC, you had the min and max constraint in the earlier response. You
> need to have rushed and posted another version before I could respond
> with my preference.
> 
> So there is no rush, these are v6.12 material. Take time for respining
> and give some time for the review.

Sure. I just not sure what the maximum should be set, so I drop
the minimum and maximum from my previous email.

Thanks,
Peng.

> 
> --
> Regards,
> Sudeep

Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Sudeep Holla]

On Thu, Jul 04, 2024 at 10:39:53AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce
> > property mbox-rx-timeout-ms
> > 
> > On Wed, Jul 03, 2024 at 11:17:14AM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > System Controller Management Interface(SCMI) firmwares might
> > have
> > > different designs by SCMI firmware developers. So the maximum
> > receive
> > > channel timeout value might also varies in the various designs.
> > >
> > > So introduce property mbox-rx-timeout-ms to let each platform could
> > > set its own timeout value in device tree.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >
> > > V2:
> > >  Drop defaults, update description.
> > >
> > >  Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6
> > ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git
> > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > index ebf384e76df1..dcac0b36c76f 100644
> > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > @@ -121,6 +121,12 @@ properties:
> > >        atomic mode of operation, even if requested.
> > >      default: 0
> > >
> > > +  max-rx-timeout-ms:
> > > +    description:
> > > +      An optional time value, expressed in milliseconds, representing
> > the
> > > +      mailbox maximum timeout value for receive channel. The value
> > should
> > > +      be a non-zero value if set.
> > > +
> > 
> > IIRC, you had the min and max constraint in the earlier response. You
> > need to have rushed and posted another version before I could respond
> > with my preference.
> > 
> > So there is no rush, these are v6.12 material. Take time for respining
> > and give some time for the review.
> 
> Sure. I just not sure what the maximum should be set, so I drop
> the minimum and maximum from my previous email.
> 

Worst case we can just have min constraint to indicate it must be non-zero
value as you have mentioned above and drop that statement as it becomes
explicit with the constraint.

-- 
Regards,
Sudeep

RE: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Peng Fan]

> Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce
> property mbox-rx-timeout-ms
> 
> On Thu, Jul 04, 2024 at 10:39:53AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > introduce property mbox-rx-timeout-ms
> > >
> > > On Wed, Jul 03, 2024 at 11:17:14AM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > System Controller Management Interface(SCMI) firmwares might
> > > have
> > > > different designs by SCMI firmware developers. So the maximum
> > > receive
> > > > channel timeout value might also varies in the various designs.
> > > >
> > > > So introduce property mbox-rx-timeout-ms to let each platform
> > > > could set its own timeout value in device tree.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >
> > > > V2:
> > > >  Drop defaults, update description.
> > > >
> > > >  Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6
> > > ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git
> > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > index ebf384e76df1..dcac0b36c76f 100644
> > > > ---
> a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > +++
> b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > @@ -121,6 +121,12 @@ properties:
> > > >        atomic mode of operation, even if requested.
> > > >      default: 0
> > > >
> > > > +  max-rx-timeout-ms:
> > > > +    description:
> > > > +      An optional time value, expressed in milliseconds,
> > > > + representing
> > > the
> > > > +      mailbox maximum timeout value for receive channel. The
> > > > + value
> > > should
> > > > +      be a non-zero value if set.
> > > > +
> > >
> > > IIRC, you had the min and max constraint in the earlier response.
> > > You need to have rushed and posted another version before I could
> > > respond with my preference.
> > >
> > > So there is no rush, these are v6.12 material. Take time for
> > > respining and give some time for the review.
> >
> > Sure. I just not sure what the maximum should be set, so I drop the
> > minimum and maximum from my previous email.
> >
> 
> Worst case we can just have min constraint to indicate it must be non-
> zero value as you have mentioned above and drop that statement as it
> becomes explicit with the constraint.

I'll use below in v3:
  max-rx-timeout-ms:                                                                                
    description:                                                                                    
      An optional time value, expressed in milliseconds, representing the                           
      mailbox maximum timeout value for receive channel. The value should                           
      be a non-zero value if set.                                                                   
    minimum: 1

Put the binding away, when you have time, please check
whether the driver changes are good or not.
BTW, since our Android team is waiting for this patchset
got R-b or A-b, then the patches could be accepted by Google common
kernel, we could support GKI in our release which is soon in near
days. So I am being pushed :) 

Thanks,
Peng

> 
> --
> Regards,
> Sudeep

Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Cristian Marussi]

On Thu, Jul 04, 2024 at 12:33:09PM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce
> > property mbox-rx-timeout-ms
> > 
> > On Thu, Jul 04, 2024 at 10:39:53AM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > > introduce property mbox-rx-timeout-ms
> > > >
> > > > On Wed, Jul 03, 2024 at 11:17:14AM +0800, Peng Fan (OSS) wrote:
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > System Controller Management Interface(SCMI) firmwares might
> > > > have
> > > > > different designs by SCMI firmware developers. So the maximum
> > > > receive
> > > > > channel timeout value might also varies in the various designs.
> > > > >
> > > > > So introduce property mbox-rx-timeout-ms to let each platform
> > > > > could set its own timeout value in device tree.
> > > > >
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > ---
> > > > >
> > > > > V2:
> > > > >  Drop defaults, update description.
> > > > >
> > > > >  Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6
> > > > ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git
> > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > index ebf384e76df1..dcac0b36c76f 100644
> > > > > ---
> > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > +++
> > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > @@ -121,6 +121,12 @@ properties:
> > > > >        atomic mode of operation, even if requested.
> > > > >      default: 0
> > > > >
> > > > > +  max-rx-timeout-ms:
> > > > > +    description:
> > > > > +      An optional time value, expressed in milliseconds,
> > > > > + representing
> > > > the
> > > > > +      mailbox maximum timeout value for receive channel. The
> > > > > + value
> > > > should
> > > > > +      be a non-zero value if set.
> > > > > +
> > > >
> > > > IIRC, you had the min and max constraint in the earlier response.
> > > > You need to have rushed and posted another version before I could
> > > > respond with my preference.
> > > >
> > > > So there is no rush, these are v6.12 material. Take time for
> > > > respining and give some time for the review.
> > >
> > > Sure. I just not sure what the maximum should be set, so I drop the
> > > minimum and maximum from my previous email.
> > >
> > 
> > Worst case we can just have min constraint to indicate it must be non-
> > zero value as you have mentioned above and drop that statement as it
> > becomes explicit with the constraint.
> 
> I'll use below in v3:
>   max-rx-timeout-ms:                                                                                
>     description:                                                                                    
>       An optional time value, expressed in milliseconds, representing the                           
>       mailbox maximum timeout value for receive channel. The value should                           
>       be a non-zero value if set.                                                                   
>     minimum: 1
> 
> Put the binding away, when you have time, please check
> whether the driver changes are good or not.
> BTW, since our Android team is waiting for this patchset
> got R-b or A-b, then the patches could be accepted by Google common
> kernel, we could support GKI in our release which is soon in near
> days. So I am being pushed :) 

Hi Peng,

once the bindings are accepted I wanted to fold also this series of
yours in my transport rework series.

Thanks,
Cristian

RE: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Peng Fan]

> Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce
> property mbox-rx-timeout-ms
> 
> On Thu, Jul 04, 2024 at 12:33:09PM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > introduce property mbox-rx-timeout-ms
> > >
> > > On Thu, Jul 04, 2024 at 10:39:53AM +0000, Peng Fan wrote:
> > > > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > > > introduce property mbox-rx-timeout-ms
> > > > >
> > > > > On Wed, Jul 03, 2024 at 11:17:14AM +0800, Peng Fan (OSS)
> wrote:
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > >
> > > > > > System Controller Management Interface(SCMI) firmwares
> might
> > > > > have
> > > > > > different designs by SCMI firmware developers. So the
> maximum
> > > > > receive
> > > > > > channel timeout value might also varies in the various designs.
> > > > > >
> > > > > > So introduce property mbox-rx-timeout-ms to let each
> platform
> > > > > > could set its own timeout value in device tree.
> > > > > >
> > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > ---
> > > > > >
> > > > > > V2:
> > > > > >  Drop defaults, update description.
> > > > > >
> > > > > >  Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> | 6
> > > > > ++++++
> > > > > >  1 file changed, 6 insertions(+)
> > > > > >
> > > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > >
> b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > index ebf384e76df1..dcac0b36c76f 100644
> > > > > > ---
> > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > +++
> > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > @@ -121,6 +121,12 @@ properties:
> > > > > >        atomic mode of operation, even if requested.
> > > > > >      default: 0
> > > > > >
> > > > > > +  max-rx-timeout-ms:
> > > > > > +    description:
> > > > > > +      An optional time value, expressed in milliseconds,
> > > > > > + representing
> > > > > the
> > > > > > +      mailbox maximum timeout value for receive channel. The
> > > > > > + value
> > > > > should
> > > > > > +      be a non-zero value if set.
> > > > > > +
> > > > >
> > > > > IIRC, you had the min and max constraint in the earlier response.
> > > > > You need to have rushed and posted another version before I
> > > > > could respond with my preference.
> > > > >
> > > > > So there is no rush, these are v6.12 material. Take time for
> > > > > respining and give some time for the review.
> > > >
> > > > Sure. I just not sure what the maximum should be set, so I drop
> > > > the minimum and maximum from my previous email.
> > > >
> > >
> > > Worst case we can just have min constraint to indicate it must be
> > > non- zero value as you have mentioned above and drop that
> statement
> > > as it becomes explicit with the constraint.
> >
> > I'll use below in v3:
> >   max-rx-timeout-ms:
> >     description:
> >       An optional time value, expressed in milliseconds, representing
> the
> >       mailbox maximum timeout value for receive channel. The value
> should
> >       be a non-zero value if set.
> >     minimum: 1
> >
> > Put the binding away, when you have time, please check whether the
> > driver changes are good or not.
> > BTW, since our Android team is waiting for this patchset got R-b or
> > A-b, then the patches could be accepted by Google common kernel,
> we
> > could support GKI in our release which is soon in near days. So I am
> > being pushed :)
> 
> Hi Peng,
> 
> once the bindings are accepted I wanted to fold also this series of yours
> in my transport rework series.

No problem, feel free to take it into your series, I will post out V3 later(wait
if Sudeep is ok with I add minimum 1 or not), but v3 2/2 should be same
as v2 2/2.

Thanks,
Peng.
> 
> Thanks,
> Cristian

Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Cristian Marussi]

On Thu, Jul 04, 2024 at 11:48:31PM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce
> > property mbox-rx-timeout-ms
> > 
> > On Thu, Jul 04, 2024 at 12:33:09PM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > > introduce property mbox-rx-timeout-ms
> > > >
> > > > On Thu, Jul 04, 2024 at 10:39:53AM +0000, Peng Fan wrote:
> > > > > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > > > > introduce property mbox-rx-timeout-ms
> > > > > >
> > > > > > On Wed, Jul 03, 2024 at 11:17:14AM +0800, Peng Fan (OSS)
> > wrote:
> > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > >
> > > > > > > System Controller Management Interface(SCMI) firmwares
> > might
> > > > > > have
> > > > > > > different designs by SCMI firmware developers. So the
> > maximum
> > > > > > receive
> > > > > > > channel timeout value might also varies in the various designs.
> > > > > > >
> > > > > > > So introduce property mbox-rx-timeout-ms to let each
> > platform
> > > > > > > could set its own timeout value in device tree.
> > > > > > >
> > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > ---
> > > > > > >
> > > > > > > V2:
> > > > > > >  Drop defaults, update description.
> > > > > > >
> > > > > > >  Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > | 6
> > > > > > ++++++
> > > > > > >  1 file changed, 6 insertions(+)
> > > > > > >
> > > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > >
> > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > > index ebf384e76df1..dcac0b36c76f 100644
> > > > > > > ---
> > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > > +++
> > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > > @@ -121,6 +121,12 @@ properties:
> > > > > > >        atomic mode of operation, even if requested.
> > > > > > >      default: 0
> > > > > > >
> > > > > > > +  max-rx-timeout-ms:
> > > > > > > +    description:
> > > > > > > +      An optional time value, expressed in milliseconds,
> > > > > > > + representing
> > > > > > the
> > > > > > > +      mailbox maximum timeout value for receive channel. The
> > > > > > > + value
> > > > > > should
> > > > > > > +      be a non-zero value if set.
> > > > > > > +
> > > > > >
> > > > > > IIRC, you had the min and max constraint in the earlier response.
> > > > > > You need to have rushed and posted another version before I
> > > > > > could respond with my preference.
> > > > > >
> > > > > > So there is no rush, these are v6.12 material. Take time for
> > > > > > respining and give some time for the review.
> > > > >
> > > > > Sure. I just not sure what the maximum should be set, so I drop
> > > > > the minimum and maximum from my previous email.
> > > > >
> > > >
> > > > Worst case we can just have min constraint to indicate it must be
> > > > non- zero value as you have mentioned above and drop that
> > statement
> > > > as it becomes explicit with the constraint.
> > >
> > > I'll use below in v3:
> > >   max-rx-timeout-ms:
> > >     description:
> > >       An optional time value, expressed in milliseconds, representing
> > the
> > >       mailbox maximum timeout value for receive channel. The value
> > should
> > >       be a non-zero value if set.
> > >     minimum: 1
> > >
> > > Put the binding away, when you have time, please check whether the
> > > driver changes are good or not.
> > > BTW, since our Android team is waiting for this patchset got R-b or
> > > A-b, then the patches could be accepted by Google common kernel,
> > we
> > > could support GKI in our release which is soon in near days. So I am
> > > being pushed :)
> > 
> > Hi Peng,
> > 
> > once the bindings are accepted I wanted to fold also this series of yours
> > in my transport rework series.
> 
> No problem, feel free to take it into your series, I will post out V3 later(wait
> if Sudeep is ok with I add minimum 1 or not), but v3 2/2 should be same
> as v2 2/2.
> 

Still not taken in transport rework V1, but not forgotten :D

Thanks,
Cristian

RE: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce property mbox-rx-timeout-ms

[Peng Fan]

> Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi: introduce
> property mbox-rx-timeout-ms
> 
> On Thu, Jul 04, 2024 at 11:48:31PM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > introduce property mbox-rx-timeout-ms
> > >
> > > On Thu, Jul 04, 2024 at 12:33:09PM +0000, Peng Fan wrote:
> > > > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > > > introduce property mbox-rx-timeout-ms
> > > > >
> > > > > On Thu, Jul 04, 2024 at 10:39:53AM +0000, Peng Fan wrote:
> > > > > > > Subject: Re: [PATCH V2 1/2] dt-bindings: firmware: arm,scmi:
> > > > > > > introduce property mbox-rx-timeout-ms
> > > > > > >
> > > > > > > On Wed, Jul 03, 2024 at 11:17:14AM +0800, Peng Fan (OSS)
> > > wrote:
> > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > >
> > > > > > > > System Controller Management Interface(SCMI) firmwares
> > > might
> > > > > > > have
> > > > > > > > different designs by SCMI firmware developers. So the
> > > maximum
> > > > > > > receive
> > > > > > > > channel timeout value might also varies in the various
> designs.
> > > > > > > >
> > > > > > > > So introduce property mbox-rx-timeout-ms to let each
> > > platform
> > > > > > > > could set its own timeout value in device tree.
> > > > > > > >
> > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > > ---
> > > > > > > >
> > > > > > > > V2:
> > > > > > > >  Drop defaults, update description.
> > > > > > > >
> > > > > > > >
> Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > | 6
> > > > > > > ++++++
> > > > > > > >  1 file changed, 6 insertions(+)
> > > > > > > >
> > > > > > > > diff --git
> > > > > > >
> a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > > >
> > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > > > index ebf384e76df1..dcac0b36c76f 100644
> > > > > > > > ---
> > > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > > > +++
> > > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > > > @@ -121,6 +121,12 @@ properties:
> > > > > > > >        atomic mode of operation, even if requested.
> > > > > > > >      default: 0
> > > > > > > >
> > > > > > > > +  max-rx-timeout-ms:
> > > > > > > > +    description:
> > > > > > > > +      An optional time value, expressed in milliseconds,
> > > > > > > > + representing
> > > > > > > the
> > > > > > > > +      mailbox maximum timeout value for receive channel.
> > > > > > > > + The value
> > > > > > > should
> > > > > > > > +      be a non-zero value if set.
> > > > > > > > +
> > > > > > >
> > > > > > > IIRC, you had the min and max constraint in the earlier
> response.
> > > > > > > You need to have rushed and posted another version before
> I
> > > > > > > could respond with my preference.
> > > > > > >
> > > > > > > So there is no rush, these are v6.12 material. Take time for
> > > > > > > respining and give some time for the review.
> > > > > >
> > > > > > Sure. I just not sure what the maximum should be set, so I
> > > > > > drop the minimum and maximum from my previous email.
> > > > > >
> > > > >
> > > > > Worst case we can just have min constraint to indicate it must
> > > > > be
> > > > > non- zero value as you have mentioned above and drop that
> > > statement
> > > > > as it becomes explicit with the constraint.
> > > >
> > > > I'll use below in v3:
> > > >   max-rx-timeout-ms:
> > > >     description:
> > > >       An optional time value, expressed in milliseconds,
> > > > representing
> > > the
> > > >       mailbox maximum timeout value for receive channel. The
> value
> > > should
> > > >       be a non-zero value if set.
> > > >     minimum: 1
> > > >
> > > > Put the binding away, when you have time, please check whether
> the
> > > > driver changes are good or not.
> > > > BTW, since our Android team is waiting for this patchset got R-b
> > > > or A-b, then the patches could be accepted by Google common
> > > > kernel,
> > > we
> > > > could support GKI in our release which is soon in near days. So I
> > > > am being pushed :)
> > >
> > > Hi Peng,
> > >
> > > once the bindings are accepted I wanted to fold also this series of
> > > yours in my transport rework series.
> >
> > No problem, feel free to take it into your series, I will post out V3
> > later(wait if Sudeep is ok with I add minimum 1 or not), but v3 2/2
> > should be same as v2 2/2.
> >
> 
> Still not taken in transport rework V1, but not forgotten :D	

No problem. just posted out v3. Only binding change to add minimum,
no more changes.

https://lore.kernel.org/all/20240709140957.3171255-1-peng.fan@oss.nxp.com/#t

Regards,
Peng.

> 
> Thanks,
> Cristian