2024-02-28 19:33:17

by Cristian Marussi

[permalink] [raw]
Subject: [PATCH 0/4] SCMI misc small-updates

Hi,

a bunch of small SCMI updates based upon sudeep/for-next/scmi/updates on
top of:

commit c2f0961a45c4 ("MAINTAINERS: Update SCMI entry with HWMON driver")

Thanks,
Cristian


Cristian Marussi (4):
include: trace: Widen the tag buffer in trace_scmi_dump_msg
firmware: arm_scmi: Add message dump traces for bad and unexpected
replies
firmware: arm_scmi: Simplify scmi_devm_notifier_unregister
firmware: arm_scmi: Use dev_err_probe to bail out

drivers/firmware/arm_scmi/driver.c | 70 ++++++++++++++++++++++++++----
drivers/firmware/arm_scmi/notify.c | 30 ++-----------
include/linux/scmi_protocol.h | 2 -
include/trace/events/scmi.h | 4 +-
4 files changed, 67 insertions(+), 39 deletions(-)

--
2.43.0



2024-02-28 19:33:21

by Cristian Marussi

[permalink] [raw]
Subject: [PATCH 1/4] include: trace: Widen the tag buffer in trace_scmi_dump_msg

A bigger buffer allow for more diverse tag names.

Signed-off-by: Cristian Marussi <[email protected]>
---
include/trace/events/scmi.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/trace/events/scmi.h b/include/trace/events/scmi.h
index 422c1ad9484d..3be75752c56f 100644
--- a/include/trace/events/scmi.h
+++ b/include/trace/events/scmi.h
@@ -150,7 +150,7 @@ TRACE_EVENT(scmi_msg_dump,
__field(u8, channel_id)
__field(u8, protocol_id)
__field(u8, msg_id)
- __array(char, tag, 5)
+ __array(char, tag, 7)
__field(u16, seq)
__field(int, status)
__field(size_t, len)
@@ -162,7 +162,7 @@ TRACE_EVENT(scmi_msg_dump,
__entry->channel_id = channel_id;
__entry->protocol_id = protocol_id;
__entry->msg_id = msg_id;
- strscpy(__entry->tag, tag, 5);
+ strscpy(__entry->tag, tag, 7);
__entry->seq = seq;
__entry->status = status;
__entry->len = len;
--
2.43.0


2024-02-28 19:33:39

by Cristian Marussi

[permalink] [raw]
Subject: [PATCH 2/4] firmware: arm_scmi: Add message dump traces for bad and unexpected replies

Use trace_scmi_msg_dump also to account for late-timed-out, out-of-order
and unexpected replies.

Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_scmi/driver.c | 36 +++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 34d77802c990..a3182199f123 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -822,6 +822,13 @@ scmi_xfer_command_acquire(struct scmi_chan_info *cinfo, u32 msg_hdr)
"Message for %d type %d is not expected!\n",
xfer_id, msg_type);
spin_unlock_irqrestore(&minfo->xfer_lock, flags);
+
+ trace_scmi_msg_dump(info->id, cinfo->id,
+ MSG_XTRACT_PROT_ID(msg_hdr),
+ MSG_XTRACT_ID(msg_hdr),
+ !msg_type ? "_!RESP" : "_!DLYD",
+ xfer_id, 0, NULL, 0);
+
return xfer;
}
refcount_inc(&xfer->users);
@@ -846,6 +853,13 @@ scmi_xfer_command_acquire(struct scmi_chan_info *cinfo, u32 msg_hdr)
dev_err(cinfo->dev,
"Invalid message type:%d for %d - HDR:0x%X state:%d\n",
msg_type, xfer_id, msg_hdr, xfer->state);
+
+ trace_scmi_msg_dump(info->id, cinfo->id,
+ MSG_XTRACT_PROT_ID(msg_hdr),
+ MSG_XTRACT_ID(msg_hdr),
+ !msg_type ? "_!RESP" : "_!DLYD",
+ xfer_id, 0, NULL, 0);
+
/* On error the refcount incremented above has to be dropped */
__scmi_xfer_put(minfo, xfer);
xfer = ERR_PTR(-EINVAL);
@@ -882,6 +896,12 @@ static void scmi_handle_notification(struct scmi_chan_info *cinfo,
if (IS_ERR(xfer)) {
dev_err(dev, "failed to get free message slot (%ld)\n",
PTR_ERR(xfer));
+
+ trace_scmi_msg_dump(info->id, cinfo->id,
+ MSG_XTRACT_PROT_ID(msg_hdr),
+ MSG_XTRACT_ID(msg_hdr), "_!NOTI",
+ MSG_XTRACT_TOKEN(msg_hdr), 0, NULL, 0);
+
scmi_clear_channel(info, cinfo);
return;
}
@@ -923,10 +943,18 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,

xfer = scmi_xfer_command_acquire(cinfo, msg_hdr);
if (IS_ERR(xfer)) {
+ u8 msg_type = MSG_XTRACT_TYPE(msg_hdr);
+
+ trace_scmi_msg_dump(info->id, cinfo->id,
+ MSG_XTRACT_PROT_ID(msg_hdr),
+ MSG_XTRACT_ID(msg_hdr),
+ !msg_type ? "_!RESP" : "_!DLYD",
+ MSG_XTRACT_TOKEN(msg_hdr), 0, NULL, 0);
+
if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT))
scmi_raw_error_report(info->raw, cinfo, msg_hdr, priv);

- if (MSG_XTRACT_TYPE(msg_hdr) == MSG_TYPE_DELAYED_RESP)
+ if (msg_type == MSG_TYPE_DELAYED_RESP)
scmi_clear_channel(info, cinfo);
return;
}
@@ -990,6 +1018,7 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv)
{
u8 msg_type = MSG_XTRACT_TYPE(msg_hdr);
+ struct scmi_info *info = handle_to_scmi_info(cinfo->handle);

switch (msg_type) {
case MSG_TYPE_NOTIFICATION:
@@ -1001,6 +1030,11 @@ void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv)
break;
default:
WARN_ONCE(1, "received unknown msg_type:%d\n", msg_type);
+ trace_scmi_msg_dump(info->id, cinfo->id,
+ MSG_XTRACT_PROT_ID(msg_hdr),
+ MSG_XTRACT_ID(msg_hdr), "_!UNKN",
+ MSG_XTRACT_TOKEN(msg_hdr), 0, NULL, 0);
+
break;
}
}
--
2.43.0


2024-02-28 19:33:46

by Cristian Marussi

[permalink] [raw]
Subject: [PATCH 3/4] firmware: arm_scmi: Simplify scmi_devm_notifier_unregister

Unregistering SCMI notifications using the managed devres interface can be
done providing as a reference simply the previously successfully registered
notification block since it could have been registered only on one kernel
notification_chain: drop any reference to SCMI protocol, events and
sources.

Devres internal helpers can search for the provided notification block
reference and, once found, the associated devres object will already
provide the above SCMI references for the event.

Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_scmi/notify.c | 30 ++++--------------------------
include/linux/scmi_protocol.h | 2 --
2 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
index 27c52531194d..e160ecb22948 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -1513,17 +1513,12 @@ static int scmi_devm_notifier_register(struct scmi_device *sdev,
static int scmi_devm_notifier_match(struct device *dev, void *res, void *data)
{
struct scmi_notifier_devres *dres = res;
- struct scmi_notifier_devres *xres = data;
+ struct notifier_block *nb = data;

- if (WARN_ON(!dres || !xres))
+ if (WARN_ON(!dres || !nb))
return 0;

- return dres->proto_id == xres->proto_id &&
- dres->evt_id == xres->evt_id &&
- dres->nb == xres->nb &&
- ((!dres->src_id && !xres->src_id) ||
- (dres->src_id && xres->src_id &&
- dres->__src_id == xres->__src_id));
+ return dres->nb == nb;
}

/**
@@ -1531,10 +1526,6 @@ static int scmi_devm_notifier_match(struct device *dev, void *res, void *data)
* notifier_block for an event
* @sdev: A reference to an scmi_device whose embedded struct device is to
* be used for devres accounting.
- * @proto_id: Protocol ID
- * @evt_id: Event ID
- * @src_id: Source ID, when NULL register for events coming form ALL possible
- * sources
* @nb: A standard notifier block to register for the specified event
*
* Generic devres managed helper to explicitly un-register a notifier_block
@@ -1544,25 +1535,12 @@ static int scmi_devm_notifier_match(struct device *dev, void *res, void *data)
* Return: 0 on Success
*/
static int scmi_devm_notifier_unregister(struct scmi_device *sdev,
- u8 proto_id, u8 evt_id,
- const u32 *src_id,
struct notifier_block *nb)
{
int ret;
- struct scmi_notifier_devres dres;
-
- dres.handle = sdev->handle;
- dres.proto_id = proto_id;
- dres.evt_id = evt_id;
- if (src_id) {
- dres.__src_id = *src_id;
- dres.src_id = &dres.__src_id;
- } else {
- dres.src_id = NULL;
- }

ret = devres_release(&sdev->dev, scmi_devm_release_notifier,
- scmi_devm_notifier_match, &dres);
+ scmi_devm_notifier_match, nb);

WARN_ON(ret);

diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 2ee94ff0320c..305d89c02a94 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -775,8 +775,6 @@ struct scmi_notify_ops {
const u32 *src_id,
struct notifier_block *nb);
int (*devm_event_notifier_unregister)(struct scmi_device *sdev,
- u8 proto_id, u8 evt_id,
- const u32 *src_id,
struct notifier_block *nb);
int (*event_notifier_register)(const struct scmi_handle *handle,
u8 proto_id, u8 evt_id,
--
2.43.0


2024-02-28 19:33:58

by Cristian Marussi

[permalink] [raw]
Subject: [PATCH 4/4] firmware: arm_scmi: Use dev_err_probe to bail out

Use dev_err_probe on the failure path of SCMI core probing.

Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_scmi/driver.c | 34 +++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index a3182199f123..7329f3a1c1de 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2522,6 +2522,10 @@ scmi_txrx_setup(struct scmi_info *info, struct device_node *of_node,
ret = 0;
}

+ if (ret)
+ dev_err(info->dev,
+ "failed to setup channel for protocol:0x%X\n", prot_id);
+
return ret;
}

@@ -2791,6 +2795,7 @@ static int scmi_debugfs_raw_mode_setup(struct scmi_info *info)
static int scmi_probe(struct platform_device *pdev)
{
int ret;
+ char *err_str = "probe failure\n";
struct scmi_handle *handle;
const struct scmi_desc *desc;
struct scmi_info *info;
@@ -2841,27 +2846,37 @@ static int scmi_probe(struct platform_device *pdev)

if (desc->ops->link_supplier) {
ret = desc->ops->link_supplier(dev);
- if (ret)
+ if (ret) {
+ err_str = "transport not ready\n";
goto clear_ida;
+ }
}

/* Setup all channels described in the DT at first */
ret = scmi_channels_setup(info);
- if (ret)
+ if (ret) {
+ err_str = "failed to setup channels\n";
goto clear_ida;
+ }

ret = bus_register_notifier(&scmi_bus_type, &info->bus_nb);
- if (ret)
+ if (ret) {
+ err_str = "failed to register bus notifier\n";
goto clear_txrx_setup;
+ }

ret = blocking_notifier_chain_register(&scmi_requested_devices_nh,
&info->dev_req_nb);
- if (ret)
+ if (ret) {
+ err_str = "failed to register device notifier\n";
goto clear_bus_notifier;
+ }

ret = scmi_xfer_info_init(info);
- if (ret)
+ if (ret) {
+ err_str = "failed to init xfers pool\n";
goto clear_dev_req_notifier;
+ }

if (scmi_top_dentry) {
info->dbg = scmi_debugfs_common_setup(info);
@@ -2898,9 +2913,11 @@ static int scmi_probe(struct platform_device *pdev)
*/
ret = scmi_protocol_acquire(handle, SCMI_PROTOCOL_BASE);
if (ret) {
- dev_err(dev, "unable to communicate with SCMI\n");
- if (coex)
+ err_str = "unable to communicate with SCMI\n";
+ if (coex) {
+ dev_err(dev, err_str);
return 0;
+ }
goto notification_exit;
}

@@ -2954,7 +2971,8 @@ static int scmi_probe(struct platform_device *pdev)
scmi_cleanup_txrx_channels(info);
clear_ida:
ida_free(&scmi_id, info->id);
- return ret;
+
+ return dev_err_probe(dev, ret, err_str);
}

static void scmi_remove(struct platform_device *pdev)
--
2.43.0