Hi,
should there be a v9, a nitpick below.
Le 02/06/2022 à 22:38, Tanmay Shah a écrit :
This driver enables r5f dual core Real time Processing Unit subsystem
available on Xilinx Zynq Ultrascale MPSoC Platform. RPU subsystem
(cluster) can be configured in different modes e.g. split mode in which
two r5f cores work independent of each other and lock-step mode in which
both r5f cores execute same code clock-for-clock and notify if the
result is different.
The Xilinx r5 Remoteproc Driver boots the RPU cores via calls to the Xilinx
Platform Management Unit that handles the R5 configuration, memory access
and R5 lifecycle management. The interface to this manager is done in this
driver via zynqmp_pm_* function calls.
Signed-off-by: Ben Levinsky <ben.levinsky-gjFFaj9aHVfQT0dZR+AlfA@xxxxxxxxxxxxxxxx>
Signed-off-by: Tanmay Shah <tanmay.shah-gjFFaj9aHVfQT0dZR+AlfA@xxxxxxxxxxxxxxxx>
---
[...]
+static void zynqmp_r5_cluster_exit(void *data)
+{
+ struct platform_device *pdev = (struct platform_device *)data;
+ struct zynqmp_r5_cluster *cluster;
+ int i;
+
+ cluster = (struct zynqmp_r5_cluster *)platform_get_drvdata(pdev);
+ if (!cluster)
+ return;
+
+ for (i = 0; i < cluster->core_count; i++) {
+ zynqmp_r5_core_exit(cluster->r5_cores[i]);
+ cluster->r5_cores[i] = NULL;
+ }
+
+ kfree(cluster->r5_cores);
+ kfree(cluster);
why not remove this kfree(cluster) here...
+ platform_set_drvdata(pdev, NULL);
+}
+
+/*
+ * zynqmp_r5_remoteproc_probe()
+ *
+ * @pdev: domain platform device for R5 cluster
+ *
+ * called when driver is probed, for each R5 core specified in DT,
+ * setup as needed to do remoteproc-related operations
+ *
+ * Return: 0 for success, negative value for failure.
+ */
+static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct zynqmp_r5_cluster *cluster;
+ struct device *dev = &pdev->dev;
+
+ cluster = kzalloc(sizeof(*cluster), GFP_KERNEL);
... devm_kzalloc() here...
+ if (!cluster)
+ return -ENOMEM;
+
+ cluster->dev = dev;
+
+ ret = devm_of_platform_populate(dev);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to populate platform dev\n");
+ kfree(cluster);
+ return ret;
and return dev_err_probe() here (without the kfree)?
Would'nt it be cleaner?
just my 2c
CJ
+ }
+
+ /* wire in so each core can be cleaned up at driver remove */
+ platform_set_drvdata(pdev, cluster);
+
+ ret = zynqmp_r5_cluster_init(cluster);
+ if (ret) {
+ zynqmp_r5_cluster_exit(pdev);
+ dev_err_probe(dev, ret, "Invalid r5f subsystem device tree\n");
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, zynqmp_r5_cluster_exit, pdev);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
[...]
