[PATCH v5 1/3] mailbox: Introduce support for T-head TH1520 Mailbox driver

[Christophe JAILLET]

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 at samsung.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