On 13/09/2023 13:16, Hari Nagalla wrote:
> From: Martyn Welch <martyn.we...@collabora.com>
> 
> 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

Reply via email to