| 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
| ||
|
Message-ID: <7b48c104-5430-455b-be11-ab23387f88ff@kernel.org>
Date: Wed, 24 Dec 2025 10:01:07 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Gaurav Kohli <gaurav.kohli@....qualcomm.com>, andersson@...nel.org,
mathieu.poirier@...aro.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, rafael@...nel.org, daniel.lezcano@...aro.org,
rui.zhang@...el.com, lukasz.luba@....com, konradybcio@...nel.org,
amitk@...nel.org, mani@...nel.org, casey.connolly@...aro.org
Cc: linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v1 4/8] thermal: qcom: add qmi-cooling driver
On 23/12/2025 13:32, Gaurav Kohli wrote:
> +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 };
> +
> + scoped_guard(mutex, &client->mutex)
> + kernel_connect(qmi->sock, (struct sockaddr_unsized *)&sq, sizeof(sq), 0);
> +
> + queue_work(system_highpri_wq, &client->svc_arrive_work);
> +
> + return 0;
> +}
> +
> +static struct qmi_ops thermal_qmi_event_ops = {
Why this is not const?
>From where did you copy this code?
> + .new_server = thermal_qmi_new_server,
> + .del_server = thermal_qmi_del_server,
> + .net_reset = thermal_qmi_net_reset,
> +};
> +
> +static void qmi_tmd_cleanup(struct qmi_tmd_client *client)
> +{
> + struct qmi_tmd *tmd, *c_next;
> +
> + guard(mutex)(&client->mutex);
> +
> + client->connection_active = false;
> +
> + qmi_handle_release(&client->handle);
> + cancel_work(&client->svc_arrive_work);
> + list_for_each_entry_safe(tmd, c_next, &client->cdev_list, node) {
> + if (tmd->rproc_cdev)
> + remoteproc_cooling_unregister(tmd->rproc_cdev);
> +
> + list_del(&tmd->node);
> + }
> +}
> +
> +/* Parse the controls and allocate a qmi_tmd for each of them */
> +static int qmi_tmd_alloc_cdevs(struct qmi_tmd_client *client)
> +{
> + struct device *dev = client->dev;
> + struct qmi_tmd *tmd;
> + struct device_node *subnode, *node = dev->of_node;
> + int ret;
> +
> + for_each_available_child_of_node(node, subnode) {
> + const char *name;
> +
> + tmd = devm_kzalloc(dev, sizeof(*tmd), GFP_KERNEL);
> + if (!tmd)
> + return dev_err_probe(client->dev, -ENOMEM,
> + "Couldn't allocate tmd\n");
You leak nodes.
> +
> + tmd->type = devm_kasprintf(client->dev, GFP_KERNEL, "%s:%s",
> + client->name, subnode->name);
> + if (!tmd->type)
> + return dev_err_probe(dev, -ENOMEM,
> + "Couldn't allocate cooling device name\n");
Everywhere...
> +
> + if (of_property_read_string(subnode, "label", &name)) {
> + return dev_err_probe(client->dev, -EINVAL,
> + "Failed to parse dev name for %s\n",
> + subnode->name);
> + }
> +
> + ret = strscpy(tmd->qmi_name, name,
> + QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1);
> + if (ret == -E2BIG) {
> + return dev_err_probe(dev, -EINVAL, "TMD label %s is too long\n",
> + name);
> + }
> +
> + tmd->client = client;
> + tmd->np = subnode;
> + tmd->cur_state = 0;
> + list_add(&tmd->node, &client->cdev_list);
> + }
> +
> + if (list_empty(&client->cdev_list))
> + return dev_err_probe(client->dev, -EINVAL,
> + "No cooling devices specified for client %s (%#x)\n",
> + client->name, client->id);
> +
> + return 0;
> +}
> +
> +static int qmi_tmd_client_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct qmi_tmd_client *client;
> + const struct qmi_instance_data *match;
> + int ret;
Open any existing Linux driver. How does this part look like?
> + 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 void qmi_tmd_client_remove(struct platform_device *pdev)
> +{
> + struct qmi_tmd_client *client = platform_get_drvdata(pdev);
> +
> + qmi_tmd_cleanup(client);
> +}
> +
> +static const struct of_device_id qmi_tmd_device_table[] = {
> + {
> + .compatible = "qcom,qmi-cooling-cdsp",
> + .data = &((struct qmi_instance_data) { CDSP_INSTANCE_ID, "cdsp" }),
Please use Linux coding style.
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, qmi_tmd_device_table);
> +
> +static struct platform_driver qmi_tmd_device_driver = {
> + .probe = qmi_tmd_client_probe,
> + .remove = qmi_tmd_client_remove,
> + .driver = {
> + .name = "qcom-qmi-cooling",
> + .of_match_table = qmi_tmd_device_table,
> + },
Best regards,
Krzysztof
Powered by blists - more mailing lists