[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <27bf1dde-ea35-4b3f-b857-4ee61959547b@wanadoo.fr>
Date: Thu, 31 Oct 2024 22:05:53 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Michal Wilczynski <m.wilczynski@...sung.com>, drew@...7.com,
guoren@...nel.org, wefu@...hat.com, jassisinghbrar@...il.com,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu,
m.szyprowski@...sung.com, samuel.holland@...ive.com,
emil.renner.berthing@...onical.com
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v5 1/3] mailbox: Introduce support for T-head TH1520
Mailbox driver
Le 31/10/2024 à 21:47, Michal Wilczynski a écrit :
> This driver was tested using the drm/imagination GPU driver. It was able
> to successfully power on the GPU, by passing a command through mailbox
> from E910 core to E902 that's responsible for powering up the GPU. The
> GPU driver was able to read the BVNC version from control registers,
> which confirms it was successfully powered on.
>
> [ 33.957467] powervr ffef400000.gpu: [drm] loaded firmware
> powervr/rogue_36.52.104.182_v1.fw
> [ 33.966008] powervr ffef400000.gpu: [drm] FW version v1.0 (build
> 6621747 OS)
> [ 38.978542] powervr ffef400000.gpu: [drm] *ERROR* Firmware failed to
> boot
>
> Though the driver still fails to boot the firmware, the mailbox driver
> works when used with the not-yet-upstreamed firmware AON driver. There
> is ongoing work to get the BXM-4-64 supported with the drm/imagination
> driver [1], though it's not completed yet.
>
> This work is based on the driver from the vendor kernel [2].
>
> Link: https://gitlab.freedesktop.org/imagination/linux-firmware/-/issues/2 [1]
> Link: https://github.com/revyos/thead-kernel.git [2]
>
> Signed-off-by: Michal Wilczynski <m.wilczynski@...sung.com>
> ---
Hi,
> +static int th1520_mbox_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct th1520_mbox_priv *priv;
> + unsigned int remote_idx = 0;
> + unsigned int i;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = dev;
> +
> + priv->clocks[0].id = "clk-local";
> + priv->clocks[1].id = "clk-remote-icu0";
> + priv->clocks[2].id = "clk-remote-icu1";
> + priv->clocks[3].id = "clk-remote-icu2";
> +
> + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(priv->clocks),
> + priv->clocks);
> + if (ret) {
> + dev_err(dev, "Failed to get clocks\n");
> + return ret;
> + }
> +
> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(priv->clocks), priv->clocks);
This should be undone if an error occurs later in the probe.
Using a devm_action_or_reset() would certainly be nicer and would avoid
the need for a .remove() function.
> + if (ret) {
> + dev_err(dev, "Failed to enable clocks\n");
> + return ret;
> + }
> +
> + /*
> + * The address mappings in the device tree align precisely with those
> + * outlined in the manual. However, register offsets within these
> + * mapped regions are irregular, particularly for remote-icu0.
> + * Consequently, th1520_map_mmio() requires an additional parameter to
> + * handle this quirk.
> + */
> + priv->local_icu[TH_1520_MBOX_ICU_KERNEL_CPU0] =
> + th1520_map_mmio(pdev, "local", 0x0);
> + if (IS_ERR(priv->local_icu[TH_1520_MBOX_ICU_KERNEL_CPU0]))
> + return PTR_ERR(priv->local_icu[TH_1520_MBOX_ICU_KERNEL_CPU0]);
> +
> + priv->remote_icu[0] = th1520_map_mmio(pdev, "remote-icu0", 0x4000);
> + if (IS_ERR(priv->remote_icu[0]))
> + return PTR_ERR(priv->remote_icu[0]);
> +
> + priv->remote_icu[1] = th1520_map_mmio(pdev, "remote-icu1", 0x0);
> + if (IS_ERR(priv->remote_icu[1]))
> + return PTR_ERR(priv->remote_icu[1]);
> +
> + priv->remote_icu[2] = th1520_map_mmio(pdev, "remote-icu2", 0x0);
> + if (IS_ERR(priv->remote_icu[2]))
> + return PTR_ERR(priv->remote_icu[2]);
> +
> + priv->local_icu[TH_1520_MBOX_ICU_CPU1] =
> + priv->local_icu[TH_1520_MBOX_ICU_KERNEL_CPU0] +
> + TH_1520_MBOX_CHAN_RES_SIZE;
> + priv->local_icu[TH_1520_MBOX_ICU_CPU2] =
> + priv->local_icu[TH_1520_MBOX_ICU_CPU1] +
> + TH_1520_MBOX_CHAN_RES_SIZE;
> + priv->local_icu[TH_1520_MBOX_ICU_CPU3] =
> + priv->local_icu[TH_1520_MBOX_ICU_CPU2] +
> + TH_1520_MBOX_CHAN_RES_SIZE;
> +
> + priv->cur_cpu_ch_base = priv->local_icu[TH_1520_MBOX_ICU_KERNEL_CPU0];
> +
> + priv->irq = platform_get_irq(pdev, 0);
> + if (priv->irq < 0)
> + return priv->irq;
> +
> + /* init the chans */
> + for (i = 0; i < TH_1520_MBOX_CHANS; i++) {
> + struct th1520_mbox_con_priv *cp = &priv->con_priv[i];
> +
> + cp->idx = i;
> + cp->chan = &priv->mbox_chans[i];
> + priv->mbox_chans[i].con_priv = cp;
> + snprintf(cp->irq_desc, sizeof(cp->irq_desc),
> + "th1520_mbox_chan[%i]", cp->idx);
> +
> + cp->comm_local_base = priv->local_icu[i];
> + if (i != TH_1520_MBOX_ICU_KERNEL_CPU0) {
> + cp->comm_remote_base = priv->remote_icu[remote_idx];
> + remote_idx++;
> + }
> + }
> +
> + spin_lock_init(&priv->mbox_lock);
> +
> + priv->mbox.dev = dev;
> + priv->mbox.ops = &th1520_mbox_ops;
> + priv->mbox.chans = priv->mbox_chans;
> + priv->mbox.num_chans = TH_1520_MBOX_CHANS;
> + priv->mbox.of_xlate = th1520_mbox_xlate;
> + priv->mbox.txdone_irq = true;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + ret = th1520_mbox_init_generic(priv);
> + if (ret) {
> + dev_err(dev, "Failed to init mailbox context\n");
> + return ret;
> + }
> +
> + return devm_mbox_controller_register(dev, &priv->mbox);
> +}
> +
> +static void th1520_mbox_remove(struct platform_device *pdev)
> +{
> + struct th1520_mbox_priv *priv = platform_get_drvdata(pdev);
> +
> + clk_bulk_disable_unprepare(ARRAY_SIZE(priv->clocks), priv->clocks);
> +}
...
CJ
Powered by blists - more mailing lists