On 9/29/23 18:16, Caleb Connolly wrote:
The Thermal Mitigation Device (TMD) service exposes various platform specific thermal mitigations available on remote subsystems (ie the modem, adsp, cdsp, and sdsp). The service is exposed via the QMI messaging interface, usually over the QRTR transport. These controls affect things like the power limits of the modem power amplifier and cpu core, skin temperature mitigations, as well as rail voltage restrictions under cold conditions. Each remote subsystem has its own TMD instance, where each child node represents a single thermal control. Signed-off-by: Caleb Connolly <caleb.connolly@xxxxxxxxxx> ---
[...]
+/* Notify the remote subsystem of the requested cooling state */ +static int qmi_tmd_send_state_request(struct qmi_tmd *tmd) +{ + struct tmd_set_mitigation_level_resp_msg_v01 tmd_resp; + struct tmd_set_mitigation_level_req_msg_v01 req; + struct qmi_tmd_client *client; + struct qmi_txn txn; + int ret = 0; + + client = tmd->client;
You can assign it at declaration time
+ + if (!client->connection_active) + return 0;
Is that an expected scenario?
+ + memset(&req, 0, sizeof(req)); + memset(&tmd_resp, 0, sizeof(tmd_resp));
Since you're declaring them above, you can do = { 0 }; instead, which will be faster
+ + strscpy(req.mitigation_dev_id.mitigation_dev_id, tmd->qmi_name, + QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1); + req.mitigation_level = tmd->cur_state; + + mutex_lock(&client->mutex);
Look into fancy scoped mutexes https://lwn.net/Articles/934679/ [...]
+static int qmi_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state) +{ + struct qmi_tmd *tmd = cdev->devdata; + + if (!tmd) + return -EINVAL; + + if (state > tmd->max_state) + return -EINVAL;Would it be useful if this threw an error in dmesg?
+ * When the QMI service starts up on a remote subsystem this function will fetch + * the list of TMDs on the subsystem, match it to the TMDs specified in devicetree + * and call qmi_tmd_init_control() for each + */ +static void qmi_tmd_svc_arrive(struct work_struct *work) +{ + struct qmi_tmd_client *client = + container_of(work, struct qmi_tmd_client, svc_arrive_work); + + struct tmd_get_mitigation_device_list_req_msg_v01 req; + struct tmd_get_mitigation_device_list_resp_msg_v01 *resp; + int ret = 0, i; + struct qmi_txn txn; + + memset(&req, 0, sizeof(req));
= { 0 } [...]
+ + pr_info("QMI TMD service reset %s\n", client->name);
Is it useful to the user? pr_debug?
+ + list_for_each_entry(tmd, &client->cdev_list, node) { + qmi_tmd_send_state_request(tmd); + } +} + +static void thermal_qmi_del_server(struct qmi_handle *qmi, struct qmi_service *service) +{ + struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle); + + pr_info("QMI TMD service stop %s\n", client->name);
Ditto
+ + client->connection_active = false; +} + +static int thermal_qmi_new_server(struct qmi_handle *qmi, struct qmi_service *service) +{ + struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle); + struct sockaddr_qrtr sq = { AF_QIPCRTR, service->node, service->port }; + + pr_info("QMI TMD service start %s\n", client->name);
Ditto
+ tmd->type = devm_kasprintf(client->dev, GFP_KERNEL, "%s:%s", + client->name, subnode->name); + if (!tmd->type) + return dev_err_probe(dev, -ENOMEM, "Cooling device name\n");
Cooling device name-what? :D
+ + if (of_property_read_string(subnode, "label", &name)) { + return dev_err_probe(client->dev, -EINVAL, + "Fail to parse dev name for %s\n",
Failed [...]
+static int qmi_tmd_client_probe(struct platform_device *pdev) +{ + struct device *dev; + struct qmi_tmd_client *client; + const struct qmi_instance_id *match; + int ret; + + dev = &pdev->dev;
Initialize at declaration
+ + client = devm_kzalloc(dev, sizeof(*client), GFP_KERNEL); + if (!client) + return -ENOMEM; + + client->dev = dev; + + match = of_device_get_match_data(dev); + if (!match) + return dev_err_probe(dev, -EINVAL, "No match data\n"); + + client->id = match->id; + client->name = match->name; + + mutex_init(&client->mutex); + INIT_LIST_HEAD(&client->cdev_list); + INIT_WORK(&client->svc_arrive_work, qmi_tmd_svc_arrive); + + ret = qmi_tmd_alloc_cdevs(client); + if (ret) + return ret; + + platform_set_drvdata(pdev, client); + + ret = qmi_handle_init(&client->handle, + TMD_GET_MITIGATION_DEVICE_LIST_RESP_MSG_V01_MAX_MSG_LEN, + &thermal_qmi_event_ops, NULL); + if (ret < 0) + return dev_err_probe(client->dev, ret, "QMI handle init failed for client %#x\n", + client->id); + + ret = qmi_add_lookup(&client->handle, TMD_SERVICE_ID_V01, TMD_SERVICE_VERS_V01, + client->id); + if (ret < 0) { + qmi_handle_release(&client->handle); + return dev_err_probe(client->dev, ret, "QMI register failed for client 0x%x\n", + client->id); + } + + return 0; +} + +static int qmi_tmd_client_remove(struct platform_device *pdev)
void + .remove_new Konrad