| Message ID | 20241018080602.3952869-6-cristian.marussi@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Expose SCMI Transport properties | expand |
On Fri, Oct 18, 2024 at 09:06:02AM +0100, Cristian Marussi wrote: > @@ -2959,7 +2952,7 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info) > (char **)&dbg->name); > > debugfs_create_u32("atomic_threshold_us", 0400, top_dentry, > - &info->atomic_threshold); > + (u32 *)&info->desc->atomic_threshold); This cast is unnecessary. > > debugfs_create_str("type", 0400, trans, (char **)&dbg->type); > > @@ -3071,6 +3064,13 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev) > trans->desc->max_rx_timeout_ms, trans->desc->max_msg_size, > trans->desc->max_msg); > > + /* System wide atomic threshold for atomic ops .. if any */ > + if (!of_property_read_u32(dev->of_node, "atomic-threshold-us", > + &trans->desc->atomic_threshold)) > + dev_info(dev, > + "SCMI System wide atomic threshold set to %d us\n", ^^ %u for unsigned int. > + trans->desc->atomic_threshold); > + > return trans->desc; > } regards, dan carpenter
On Wed, Oct 23, 2024 at 04:20:53PM +0300, Dan Carpenter wrote: > On Fri, Oct 18, 2024 at 09:06:02AM +0100, Cristian Marussi wrote: Hi Dan, thanks for having a look. > > @@ -2959,7 +2952,7 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info) > > (char **)&dbg->name); > > > > debugfs_create_u32("atomic_threshold_us", 0400, top_dentry, > > - &info->atomic_threshold); > > + (u32 *)&info->desc->atomic_threshold); > > This cast is unnecessary. I was indeed wondering why I added that....then I remember something about debugfs_create....without that (u32 *): drivers/firmware/arm_scmi/driver.c: In function ‘scmi_debugfs_common_setup’: drivers/firmware/arm_scmi/driver.c:2988:28: warning: passing argument 4 of ‘debugfs_create_u32’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] &info->desc->atomic_threshold); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ since the enclosing struct ->desc is const AND debugfs_create_u32 is NOT smart enough to expect a const when the property is R_ONLY...unless I am missing something. > > > > > debugfs_create_str("type", 0400, trans, (char **)&dbg->type); > > > > @@ -3071,6 +3064,13 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev) > > trans->desc->max_rx_timeout_ms, trans->desc->max_msg_size, > > trans->desc->max_msg); > > > > + /* System wide atomic threshold for atomic ops .. if any */ > > + if (!of_property_read_u32(dev->of_node, "atomic-threshold-us", > > + &trans->desc->atomic_threshold)) > > + dev_info(dev, > > + "SCMI System wide atomic threshold set to %d us\n", > ^^ > %u for unsigned int. > I will fix. Thanks, Cristian
On Fri, Oct 25, 2024 at 03:35:57PM +0100, Cristian Marussi wrote: > On Wed, Oct 23, 2024 at 04:20:53PM +0300, Dan Carpenter wrote: > > On Fri, Oct 18, 2024 at 09:06:02AM +0100, Cristian Marussi wrote: > > Hi Dan, > > thanks for having a look. > > > > @@ -2959,7 +2952,7 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info) > > > (char **)&dbg->name); > > > > > > debugfs_create_u32("atomic_threshold_us", 0400, top_dentry, > > > - &info->atomic_threshold); > > > + (u32 *)&info->desc->atomic_threshold); > > > > This cast is unnecessary. > > I was indeed wondering why I added that....then I remember something > about debugfs_create....without that (u32 *): > > drivers/firmware/arm_scmi/driver.c: In function ‘scmi_debugfs_common_setup’: > drivers/firmware/arm_scmi/driver.c:2988:28: warning: passing argument 4 of ‘debugfs_create_u32’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > &info->desc->atomic_threshold); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > since the enclosing struct ->desc is const AND debugfs_create_u32 is NOT > smart enough to expect a const when the property is R_ONLY...unless I am > missing something. > Ah, I missed the const. Sorry about that. regards, dan carpenter
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index d867bcc6883b..058d4b0e3de9 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -227,6 +227,12 @@ struct scmi_transport_ops { * be pending simultaneously in the system. May be overridden by the * get_max_msg op. * @max_msg_size: Maximum size of data payload per message that can be handled. + * @atomic_threshold: Optional system wide DT-configured threshold, expressed + * in microseconds, for atomic operations. + * Only SCMI synchronous commands reported by the platform + * to have an execution latency lesser-equal to the threshold + * should be considered for atomic mode operation: such + * decision is finally left up to the SCMI drivers. * @force_polling: Flag to force this whole transport to use SCMI core polling * mechanism instead of completion interrupts even if available. * @sync_cmds_completed_on_ret: Flag to indicate that the transport assures @@ -245,6 +251,7 @@ struct scmi_desc { int max_rx_timeout_ms; int max_msg; int max_msg_size; + unsigned int atomic_threshold; const bool force_polling; const bool sync_cmds_completed_on_ret; const bool atomic_enabled; diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 09eb0713d91c..c9adf91643ed 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -149,12 +149,6 @@ struct scmi_debug_info { * base protocol * @active_protocols: IDR storing device_nodes for protocols actually defined * in the DT and confirmed as implemented by fw. - * @atomic_threshold: Optional system wide DT-configured threshold, expressed - * in microseconds, for atomic operations. - * Only SCMI synchronous commands reported by the platform - * to have an execution latency lesser-equal to the threshold - * should be considered for atomic mode operation: such - * decision is finally left up to the SCMI drivers. * @notify_priv: Pointer to private data structure specific to notifications. * @node: List head * @users: Number of users of this instance @@ -180,7 +174,6 @@ struct scmi_info { struct mutex protocols_mtx; u8 *protocols_imp; struct idr active_protocols; - unsigned int atomic_threshold; void *notify_priv; struct list_head node; int users; @@ -2445,7 +2438,7 @@ static bool scmi_is_transport_atomic(const struct scmi_handle *handle, ret = info->desc->atomic_enabled && is_transport_polling_capable(info->desc); if (ret && atomic_threshold) - *atomic_threshold = info->atomic_threshold; + *atomic_threshold = info->desc->atomic_threshold; return ret; } @@ -2959,7 +2952,7 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info) (char **)&dbg->name); debugfs_create_u32("atomic_threshold_us", 0400, top_dentry, - &info->atomic_threshold); + (u32 *)&info->desc->atomic_threshold); debugfs_create_str("type", 0400, trans, (char **)&dbg->type); @@ -3071,6 +3064,13 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev) trans->desc->max_rx_timeout_ms, trans->desc->max_msg_size, trans->desc->max_msg); + /* System wide atomic threshold for atomic ops .. if any */ + if (!of_property_read_u32(dev->of_node, "atomic-threshold-us", + &trans->desc->atomic_threshold)) + dev_info(dev, + "SCMI System wide atomic threshold set to %d us\n", + trans->desc->atomic_threshold); + return trans->desc; } @@ -3120,13 +3120,6 @@ static int scmi_probe(struct platform_device *pdev) handle->devm_protocol_acquire = scmi_devm_protocol_acquire; handle->devm_protocol_get = scmi_devm_protocol_get; handle->devm_protocol_put = scmi_devm_protocol_put; - - /* System wide atomic threshold for atomic ops .. if any */ - if (!of_property_read_u32(np, "atomic-threshold-us", - &info->atomic_threshold)) - dev_info(dev, - "SCMI System wide atomic threshold set to %d us\n", - info->atomic_threshold); handle->is_transport_atomic = scmi_is_transport_atomic; /* Setup all channels described in the DT at first */
Relocate the atomic_threshold field to scmi_desc and move the related code to scmi_transport_setup. No functional change. Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> --- drivers/firmware/arm_scmi/common.h | 7 +++++++ drivers/firmware/arm_scmi/driver.c | 25 +++++++++---------------- 2 files changed, 16 insertions(+), 16 deletions(-)