All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: tanmay.shah@amd.com
Cc: linux-remoteproc@vger.kernel.org
Subject: [bug report] drivers: remoteproc: Add Xilinx r5 remoteproc driver
Date: Wed, 24 Apr 2024 14:43:52 +0300	[thread overview]
Message-ID: <77483e55-d015-4444-b6ee-95fbfb226fde@moroto.mountain> (raw)

Hello Tanmay Shah,

Commit 6b291e8020a8 ("drivers: remoteproc: Add Xilinx r5 remoteproc
driver") from Nov 14, 2022 (linux-next), leads to the following
Smatch static checker warning:

    drivers/remoteproc/xlnx_r5_remoteproc.c:1088 zynqmp_r5_cluster_init()
    error: uninitialized symbol 'tcm_mode'.

    drivers/remoteproc/xlnx_r5_remoteproc.c:917 zynqmp_r5_core_init()
    error: uninitialized symbol 'ret'.

drivers/remoteproc/xlnx_r5_remoteproc.c
    961 static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
    962 {
    963         enum zynqmp_r5_cluster_mode cluster_mode = LOCKSTEP_MODE;
    964         struct device *dev = cluster->dev;
    965         struct device_node *dev_node = dev_of_node(dev);
    966         struct platform_device *child_pdev;
    967         struct zynqmp_r5_core **r5_cores;
    968         enum rpu_oper_mode fw_reg_val;
    969         struct device **child_devs;
    970         struct device_node *child;
    971         enum rpu_tcm_comb tcm_mode;
    972         int core_count, ret, i;
    973         struct mbox_info *ipi;
    974 
    975         ret = of_property_read_u32(dev_node, "xlnx,cluster-mode", &cluster_mode);
    976 
    977         /*
    978          * on success returns 0, if not defined then returns -EINVAL,
    979          * In that case, default is LOCKSTEP mode. Other than that
    980          * returns relative error code < 0.
    981          */
    982         if (ret != -EINVAL && ret != 0) {
    983                 dev_err(dev, "Invalid xlnx,cluster-mode property\n");
    984                 return ret;
    985         }
    986 
    987         /*
    988          * For now driver only supports split mode and lockstep mode.
    989          * fail driver probe if either of that is not set in dts.
    990          */
    991         if (cluster_mode == LOCKSTEP_MODE) {
    992                 fw_reg_val = PM_RPU_MODE_LOCKSTEP;
    993         } else if (cluster_mode == SPLIT_MODE) {
    994                 fw_reg_val = PM_RPU_MODE_SPLIT;
    995         } else {
    996                 dev_err(dev, "driver does not support cluster mode %d\n", cluster_mode);
    997                 return -EINVAL;
    998         }
    999 
    1000         if (of_find_property(dev_node, "xlnx,tcm-mode", NULL)) {
    1001                 ret = of_property_read_u32(dev_node, "xlnx,tcm-mode", (u32 *)&tcm_mode);
    1002                 if (ret)
    1003                         return ret;
    1004         } else if (device_is_compatible(dev, "xlnx,zynqmp-r5fss")) {
    1005                 if (cluster_mode == LOCKSTEP_MODE)
    1006                         tcm_mode = PM_RPU_TCM_COMB;
    1007                 else
    1008                         tcm_mode = PM_RPU_TCM_SPLIT;
    1009         }

tcm_mode not initialized if device_is_compatible() is false.

    1010 
    1011         /*
    1012          * Number of cores is decided by number of child nodes of
    1013          * r5f subsystem node in dts. If Split mode is used in dts
    1014          * 2 child nodes are expected.
    1015          * In lockstep mode if two child nodes are available,
    1016          * only use first child node and consider it as core0
    1017          * and ignore core1 dt node.
    1018          */
    1019         core_count = of_get_available_child_count(dev_node);
    1020         if (core_count == 0) {
    1021                 dev_err(dev, "Invalid number of r5 cores %d", core_count);
    1022                 return -EINVAL;
    1023         } else if (cluster_mode == SPLIT_MODE && core_count != 2) {
    1024                 dev_err(dev, "Invalid number of r5 cores for split mode\n");
    1025                 return -EINVAL;
    1026         } else if (cluster_mode == LOCKSTEP_MODE && core_count == 2) {
    1027                 dev_warn(dev, "Only r5 core0 will be used\n");
    1028                 core_count = 1;
    1029         }
    1030 
    1031         child_devs = kcalloc(core_count, sizeof(struct device *), GFP_KERNEL);
    1032         if (!child_devs)
    1033                 return -ENOMEM;
    1034 
    1035         r5_cores = kcalloc(core_count,
    1036                            sizeof(struct zynqmp_r5_core *), GFP_KERNEL);
    1037         if (!r5_cores) {
    1038                 kfree(child_devs);
    1039                 return -ENOMEM;
    1040         }
    1041 
    1042         i = 0;
    1043         for_each_available_child_of_node(dev_node, child) {
    1044                 child_pdev = of_find_device_by_node(child);
    1045                 if (!child_pdev) {
    1046                         of_node_put(child);
    1047                         ret = -ENODEV;
    1048                         goto release_r5_cores;
    1049                 }
    1050 
    1051                 child_devs[i] = &child_pdev->dev;
    1052 
    1053                 /* create and add remoteproc instance of type struct rproc */
    1054                 r5_cores[i] = zynqmp_r5_add_rproc_core(&child_pdev->dev);
    1055                 if (IS_ERR(r5_cores[i])) {
    1056                         of_node_put(child);
    1057                         ret = PTR_ERR(r5_cores[i]);
    1058                         r5_cores[i] = NULL;
    1059                         goto release_r5_cores;
    1060                 }
    1061 
    1062                 /*
    1063                  * If mailbox nodes are disabled using "status" property then
    1064                  * setting up mailbox channels will fail.
    1065                  */
    1066                 ipi = zynqmp_r5_setup_mbox(&child_pdev->dev);
    1067                 if (ipi) {
    1068                         r5_cores[i]->ipi = ipi;
    1069                         ipi->r5_core = r5_cores[i];
    1070                 }
    1071 
    1072                 /*
    1073                  * If two child nodes are available in dts in lockstep mode,
    1074                  * then ignore second child node.
    1075                  */
    1076                 if (cluster_mode == LOCKSTEP_MODE) {
    1077                         of_node_put(child);
    1078                         break;
    1079                 }
    1080 
    1081                 i++;
    1082         }
    1083 
    1084         cluster->mode = cluster_mode;
    1085         cluster->core_count = core_count;
    1086         cluster->r5_cores = r5_cores;
    1087 
--> 1088         ret = zynqmp_r5_core_init(cluster, fw_reg_val, tcm_mode);
    1089         if (ret < 0) {
    1090                 dev_err(dev, "failed to init r5 core err %d\n", ret);
    1091                 cluster->core_count = 0;
    1092                 cluster->r5_cores = NULL;
    1093 
    1094                 /*
    1095                  * at this point rproc resources for each core are allocated.
    1096                  * adjust index to free resources in reverse order
    1097                  */
    1098                 i = core_count - 1;
    1099                 goto release_r5_cores;
    1100         }
    1101 
    1102         kfree(child_devs);
    1103         return 0;
    1104 
    1105 release_r5_cores:
    1106         while (i >= 0) {
    1107                 put_device(child_devs[i]);
    1108                 if (r5_cores[i]) {
    1109                         zynqmp_r5_free_mbox(r5_cores[i]->ipi);
    1110                         of_reserved_mem_device_release(r5_cores[i]->dev);
    1111                         rproc_del(r5_cores[i]->rproc);
    1112                         rproc_free(r5_cores[i]->rproc);
    1113                 }
    1114                 i--;
    1115         }
    1116         kfree(r5_cores);
    1117         kfree(child_devs);
    1118         return ret;
    1119 }

regards,
dan carpenter

             reply	other threads:[~2024-04-24 11:43 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-24 11:43 Dan Carpenter [this message]
2024-04-24 16:05 ` [bug report] drivers: remoteproc: Add Xilinx r5 remoteproc driver Tanmay Shah

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=77483e55-d015-4444-b6ee-95fbfb226fde@moroto.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=tanmay.shah@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.