On 7/19/2023 12:56 PM, Krzysztof Kozlowski wrote:
On 18/07/2023 14:04, Manikanta Mylavarapu wrote:
It adds support to bring up remoteproc's on multipd model.
Pd means protection domain. It's similar to process in Linux.
Here QDSP6 processor runs each wifi radio functionality on a
separate process. One process can't access other process
resources, so this is termed as PD i.e protection domain.
Here we have two pd's called root and user pd. We can correlate
Root pd as root and user pd as user in linux. Root pd has more
privileges than user pd. Root will provide services to user pd.
+static int q6_get_inbound_irq(struct qcom_q6v5 *q6,
+ struct platform_device *pdev,
+ const char *int_name,
+ int index, int *pirq,
+ irqreturn_t (*handler)(int irq, void *data))
+{
+ int ret, irq;
+ char *interrupt, *tmp = (char *)int_name;
+ struct q6_wcss *wcss = q6->rproc->priv;
+
+ irq = platform_get_irq(pdev, index);
+ if (irq < 0) {
+ if (irq != -EPROBE_DEFER)
+ dev_err_probe(&pdev->dev, irq,
+ "failed to retrieve %s IRQ: %d\n",
+ int_name, irq);
Wait, what? This does not make any sense. dev_err_probe is to replace
all this dance. return dev_err_probe which I explicitly asked last time:
>
https://lore.kernel.org/all/2061a641-4b97-1aa6-27cd-99f01a785033@xxxxxxxxxx/
Yeah i got your point. I will return dev_err_probe alone.
+ return irq;
+ }
+
+ *pirq = irq;
+
+ interrupt = devm_kzalloc(&pdev->dev, BUF_SIZE, GFP_KERNEL);
+ if (!interrupt)
+ return -ENOMEM;
+
+ snprintf(interrupt, BUF_SIZE, "q6v5_wcss_userpd%d_%s", wcss->pd_asid, tmp);
+
+ ret = devm_request_threaded_irq(&pdev->dev, *pirq,
+ NULL, handler,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ interrupt, q6);
+ if (ret) {
+ dev_err_probe(&pdev->dev, ret,
+ "failed to acquire %s irq\n", interrupt);
+ return ret;
+ }
+ return 0;
+}
+
+static int q6_get_outbound_irq(struct qcom_q6v5 *q6,
+ struct platform_device *pdev,
+ const char *int_name)
+{
+ struct qcom_smem_state *tmp_state;
+ unsigned bit;
+
+ tmp_state = qcom_smem_state_get(&pdev->dev, int_name, &bit);
+ if (IS_ERR(tmp_state)) {
+ dev_err_probe(&pdev->dev, IS_ERR(tmp_state),
+ "failed to acquire %s state\n", int_name);
+ return PTR_ERR(tmp_state);
So it is everywhere...
Wherever applicable i will return dev_err_probe.
+ }
+
+ if (!strcmp(int_name, "stop")) {
+ q6->state = tmp_state;
+ q6->stop_bit = bit;
+ } else if (!strcmp(int_name, "spawn")) {
+ q6->spawn_state = tmp_state;
+ q6->spawn_bit = bit;
+ }
+
+ return 0;
+}
+
+static int init_irq(struct qcom_q6v5 *q6,
+ struct platform_device *pdev, struct rproc *rproc,
+ int crash_reason, const char *load_state,
+ void (*handover)(struct qcom_q6v5 *q6))
+{
+ int ret;
+ struct q6_wcss *wcss = rproc->priv;
+
+ q6->rproc = rproc;
+ q6->dev = &pdev->dev;
+ q6->crash_reason = crash_reason;
+ q6->handover = handover;
+
+ init_completion(&q6->start_done);
+ init_completion(&q6->stop_done);
+ init_completion(&q6->spawn_done);
+
+ ret = q6_get_outbound_irq(q6, pdev, "stop");
+ if (ret)
+ return ret;
+
+ ret = q6_get_outbound_irq(q6, pdev, "spawn");
+ if (ret)
+ return ret;
+
+ /* Get pd_asid to prepare interrupt names */
+ wcss->pd_asid = qcom_get_pd_asid(rproc);
+
+ ret = q6_get_inbound_irq(q6, pdev, "fatal", 0, &q6->fatal_irq,
+ q6v5_fatal_interrupt);
+ if (ret)
+ return ret;
+
+ ret = q6_get_inbound_irq(q6, pdev, "ready", 1, &q6->ready_irq,
+ q6v5_ready_interrupt);
+ if (ret)
+ return ret;
+
+ ret = q6_get_inbound_irq(q6, pdev, "stop-ack", 3, &q6->stop_irq,
+ q6v5_stop_interrupt);
+ if (ret)
+ return ret;
+
+ ret = q6_get_inbound_irq(q6, pdev, "spawn-ack", 2, &q6->spawn_irq,
+ q6v5_spawn_interrupt);
+ if (ret)
+ return ret;
+ return 0;
+}
+
+static void q6_release_resources(struct platform_device *pdev)
+{
+ struct rproc *upd_rproc;
+ struct device_node *upd_np;
+ struct platform_device *upd_pdev;
+
+ /* Release userpd resources */
+ for_each_available_child_of_node(pdev->dev.of_node, upd_np) {
You should not iterate over OF to get devices to unregister. What if you
have more nodes than before because of overlay?
+ upd_pdev = of_find_device_by_node(upd_np);
+ if (!upd_pdev)
+ continue;
+
+ upd_rproc = platform_get_drvdata(upd_pdev);
+ if (!upd_rproc) {
+ platform_device_unregister(upd_pdev);
+ continue;
+ }
+
+ rproc_del(upd_rproc);
+ rproc_free(upd_rproc);
+ }
+}
+
+static int q6_register_userpd(struct platform_device *pdev)
+{
+ struct q6_wcss *wcss;
+ struct rproc *rproc = NULL;
+ int ret;
+ struct device_node *userpd_np;
+ struct platform_device *userpd_pdev;
+ const char *firmware_name = NULL;
+
+ for_each_available_child_of_node(pdev->dev.of_node, userpd_np) {
+ ret = of_property_read_string(userpd_np, "firmware-name",
+ &firmware_name);
+ if (ret < 0)
+ continue;
+
+ dev_info(&pdev->dev, "%s node found\n", userpd_np->name);
+
+ userpd_pdev = of_platform_device_create(userpd_np,
+ userpd_np->name,
+ &pdev->dev);
+ if (!userpd_pdev) {
+ ret = -ENODEV;
+ dev_err_probe(&pdev->dev, ret,
+ "failed to create %s platform device\n",
+ userpd_np->name);
+ goto release_resource;
+ }
+ userpd_pdev->dev.driver = pdev->dev.driver;
+ rproc = rproc_alloc(&userpd_pdev->dev, userpd_pdev->name,
+ &wcss_ops, firmware_name,
+ sizeof(*wcss));
+ if (!rproc) {
+ ret = -ENOMEM;
+ goto release_resource;
+ }
+
+ wcss = rproc->priv;
+ wcss->dev = &userpd_pdev->dev;
+
+ ret = q6_alloc_memory_region(wcss);
+ if (ret)
How do you release the resource allocated in rproc_alloc() for this
node? drvdata is not set, so your cleanup function will skip it.
+ goto release_resource;
+
+ ret = init_irq(&wcss->q6, userpd_pdev, rproc,
+ WCSS_CRASH_REASON, NULL, NULL);
+ if (ret)
+ goto release_resource;
+
+ rproc->auto_boot = false;
+ ret = rproc_add(rproc);
+ if (ret)
+ goto release_resource;
+
+ platform_set_drvdata(userpd_pdev, rproc);
+ qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, userpd_pdev->name);
+ }
+ return 0;
+
+release_resource:
+ q6_release_resources(pdev);
+ return ret;
+}
+
+static int q6_wcss_probe(struct platform_device *pdev)
+{
+ const struct wcss_data *desc;
+ struct q6_wcss *wcss;
+ struct rproc *rproc;
+ int ret;
+ char *subdev_name;
+ const char **firmware;
+
+ desc = of_device_get_match_data(&pdev->dev);
+ if (!desc)
+ return -EINVAL;
+
+ firmware = devm_kcalloc(&pdev->dev, MAX_FIRMWARE,
+ sizeof(*firmware), GFP_KERNEL);
+ if (!firmware)
+ return -ENOMEM;
+
+ ret = of_property_read_string_array(pdev->dev.of_node, "firmware-name",
+ firmware, MAX_FIRMWARE);
+ if (ret < 0)
+ return ret;
+
+ rproc = rproc_alloc(&pdev->dev, pdev->name, desc->ops,
+ firmware[0], sizeof(*wcss));
+ if (!rproc)
+ return -ENOMEM;
+
+ wcss = rproc->priv;
+ wcss->dev = &pdev->dev;
+ wcss->desc = desc;
+ wcss->firmware = firmware;
+
+ ret = q6_alloc_memory_region(wcss);
+ if (ret)
+ goto free_rproc;
+
+ ret = desc->init_irq(&wcss->q6, pdev, rproc,
+ desc->crash_reason_smem, NULL, NULL);
+ if (ret)
+ goto free_rproc;
+
+ if (desc->glink_subdev_required)
+ qcom_add_glink_subdev(rproc, &wcss->glink_subdev, desc->ssr_name);
+
+ subdev_name = (char *)(desc->ssr_name ? desc->ssr_name : pdev->name);
Wrong cast. Why are you dropping const? That's a bug.
Yeah, I will use desc->ssr_name itself.
+ qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, subdev_name);
+
+ rproc->auto_boot = false;
+ ret = rproc_add(rproc);
+ if (ret)
+ goto free_rproc;
+
+ platform_set_drvdata(pdev, rproc);
+
+ ret = q6_register_userpd(pdev);
+ if (ret) {
+ dev_err_probe(&pdev->dev, ret, "Failed to register userpd\n");
+ return ret;
return dev_err_probe
ok, sure.
+ }
+
+ return 0;
+
+free_rproc:
+ rproc_free(rproc);
+
+ return ret;
Thanks & Regards,
Manikanta.
