Hi Mathieu

On Thu, Oct 7, 2021 at 12:25 AM Mathieu Poirier
[off-list ref] wrote:

Hi Shengjiu,

This pachset doesn't apply to rproc-next, which is now located here[1].  The
change is in linux-next but not in mainline yet.

https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/log/?h=rproc-next

Ok, I will double check it and fix it.

On Sun, Sep 26, 2021 at 11:07:09AM +0800, Shengjiu Wang wrote:

quoted

Provide a basic driver to control DSP processor found on NXP i.MX8QM,
i.MX8QXP, i.MX8MP and i.MX8ULP.

Currently it is able to resolve addresses between DSP and main CPU,
start and stop the processor, suspend and resume.

The communication between DSP and main CPU is based on mailbox, there
are three mailbox channels (tx, rx, rxdb).

This driver was tested on NXP i.MX8QM, i.MX8QXP, i.MX8MP and i.MX8ULP.

Signed-off-by: Shengjiu Wang <redacted>
---
 drivers/remoteproc/Kconfig         |   11 +
 drivers/remoteproc/Makefile        |    1 +
 drivers/remoteproc/imx_dsp_rproc.c | 1206 ++++++++++++++++++++++++++++
 3 files changed, 1218 insertions(+)
 create mode 100644 drivers/remoteproc/imx_dsp_rproc.c

[...]

quoted

+
+/**
+ * imx_dsp_attach_pm_domains() - attach the power domains
+ * @priv: private data pointer
+ *
+ * On i.MX8QM and i.MX8QXP there is multiple power domains
+ * required, so need to link them.
+ */
+static int imx_dsp_attach_pm_domains(struct imx_dsp_rproc *priv)
+{
+     struct device *dev = priv->rproc->dev.parent;
+     int ret, i;
+
+     priv->num_domains = of_count_phandle_with_args(dev->of_node,
+                                                    "power-domains",
+                                                    "#power-domain-cells");
+
+     /* If only one domain, then no need to link the device */
+     if (priv->num_domains <= 1)
+             return 0;
+
+     priv->pd_dev = devm_kmalloc_array(dev, priv->num_domains,
+                                       sizeof(*priv->pd_dev),
+                                       GFP_KERNEL);
+     if (!priv->pd_dev)
+             return -ENOMEM;
+
+     priv->pd_dev_link = devm_kmalloc_array(dev, priv->num_domains,
+                                            sizeof(*priv->pd_dev_link),
+                                            GFP_KERNEL);
+     if (!priv->pd_dev_link)
+             return -ENOMEM;
+
+     for (i = 0; i < priv->num_domains; i++) {
+             priv->pd_dev[i] = dev_pm_domain_attach_by_id(dev, i);
+             if (IS_ERR(priv->pd_dev[i])) {
+                     ret = PTR_ERR(priv->pd_dev[i]);
+                     goto detach_pm;
+             }

I have pointed a problem with the error handling in the above during the
previous review and it was not addressed.

I have considered your comments.  Actually when
dev_pm_domain_attach_by_id() return NULL, the device_link_add()
will break, I have added comments below, so above error handling
for dev_pm_domain_attach_by_id() is enough.

Best regards
Wang Shengjiu

quoted

+
+             /*
+              * device_link_add will check priv->pd_dev[i], if it is
+              * NULL, then will break.
+              */
+             priv->pd_dev_link[i] = device_link_add(dev,
+                                                    priv->pd_dev[i],
+                                                    DL_FLAG_STATELESS |
+                                                    DL_FLAG_PM_RUNTIME);
+             if (!priv->pd_dev_link[i]) {
+                     dev_pm_domain_detach(priv->pd_dev[i], false);
+                     ret = -EINVAL;
+                     goto detach_pm;
+             }
+     }
+
+     return 0;
+
+detach_pm:
+     while (--i >= 0) {
+             device_link_del(priv->pd_dev_link[i]);
+             dev_pm_domain_detach(priv->pd_dev[i], false);
+     }
+
+     return ret;
+}
+

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel