On 13/09/2023 13:16, Hari Nagalla wrote:
> From: Martyn Welch <martyn.welch@xxxxxxxxxxxxx>
>
> The AM62x and AM64x SoCs of the TI K3 family has a Cortex M4F core in
> the MCU domain. This core is typically used for safety applications in a
> stand alone mode. However, some application (non safety related) may
> want to use the M4F core as a generic remote processor with IPC to the
> host processor. The M4F core has internal IRAM and DRAM memories and are
> exposed to the system bus for code and data loading.
>
> drivers/remoteproc/Kconfig | 13 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/ti_k3_m4_remoteproc.c | 331 +++++++++++++++++++++++
> 3 files changed, 345 insertions(+)
> create mode 100644 drivers/remoteproc/ti_k3_m4_remoteproc.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 48845dc8fa85..85c1a3a2b987 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -339,6 +339,19 @@ config TI_K3_DSP_REMOTEPROC
> It's safe to say N here if you're not interested in utilizing
> the DSP slave processors.
>
> +config TI_K3_M4_REMOTEPROC
> + tristate "TI K3 M4 remoteproc support"
> + depends on ARCH_K3
Missing compile testing.
...
> +
> +static int k3_m4_rproc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + const struct k3_rproc_dev_data *data;
> + struct k3_rproc *kproc;
> + struct rproc *rproc;
> + const char *fw_name;
> + bool r_state = false;
> + bool p_state = false;
> + int ret = 0;
> + int ret1;
> +
> + data = of_device_get_match_data(dev);
> + if (!data)
> + return -ENODEV;
> +
> + ret = rproc_of_parse_firmware(dev, 0, &fw_name);
> + if (ret) {
> + dev_err(dev, "failed to parse firmware-name property, ret = %d\n",
> + ret);
> + return ret;
Nope, the syntax is dev_err_probe().
> + }
> +
> + rproc = rproc_alloc(dev, dev_name(dev), &k3_m4_rproc_ops, fw_name,
> + sizeof(*kproc));
> + if (!rproc)
> + return -ENOMEM;
> +
> + rproc->has_iommu = false;
> + rproc->recovery_disabled = true;
> + if (data->uses_lreset) {
> + rproc->ops->prepare = k3_rproc_prepare;
> + rproc->ops->unprepare = k3_rproc_unprepare;
> + }
> + kproc = rproc->priv;
> + kproc->rproc = rproc;
> + kproc->dev = dev;
> + kproc->data = data;
> +
> + kproc->ti_sci = ti_sci_get_by_phandle(np, "ti,sci");
> + if (IS_ERR(kproc->ti_sci)) {
> + ret = PTR_ERR(kproc->ti_sci);
> + if (ret != -EPROBE_DEFER) {
No, really, do not open-code existing code.
> + dev_err(dev, "failed to get ti-sci handle, ret = %d\n",
> + ret);
> + }
> + kproc->ti_sci = NULL;
> + goto free_rproc;
> + }
> +
> + ret = of_property_read_u32(np, "ti,sci-dev-id", &kproc->ti_sci_id);
> + if (ret) {
> + dev_err(dev, "missing 'ti,sci-dev-id' property\n");
> + goto put_sci;
> + }
> +
> + kproc->reset = devm_reset_control_get_exclusive(dev, NULL);
> + if (IS_ERR(kproc->reset)) {
> + ret = PTR_ERR(kproc->reset);
> + dev_err(dev, "failed to get reset, status = %d\n", ret);
Syntax is return dev_err_probe. And everywhere else as well...
Best regards,
Krzysztof
