On Fri, Sep 27, 2024 at 11:42:05AM +0200, Michal Wilczynski wrote: > 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 BVC 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@xxxxxxxxxxx> > --- > MAINTAINERS | 1 + > drivers/mailbox/Kconfig | 10 + > drivers/mailbox/Makefile | 2 + > drivers/mailbox/mailbox-th1520.c | 551 +++++++++++++++++++++++++++++++ > 4 files changed, 564 insertions(+) > create mode 100644 drivers/mailbox/mailbox-th1520.c ... > +static int th1520_mbox_startup(struct mbox_chan *chan) > +{ > + struct th1520_mbox_priv *priv = to_th1520_mbox_priv(chan->mbox); > + struct th1520_mbox_con_priv *cp = chan->con_priv; > + u32 data[8] = {}; > + int mask_bit; > + int ret; > + > + /* clear local and remote generate and info0~info7 */ > + th1520_mbox_chan_rmw(cp, TH_1520_MBOX_GEN, 0x0, 0xff, true); > + th1520_mbox_chan_rmw(cp, TH_1520_MBOX_GEN, 0x0, 0xff, false); > + th1520_mbox_chan_wr_ack(cp, &data[7], true); > + th1520_mbox_chan_wr_ack(cp, &data[7], false); > + th1520_mbox_chan_wr_data(cp, &data[0], true); > + th1520_mbox_chan_wr_data(cp, &data[0], false); > + > + /* enable the chan mask */ > + mask_bit = th1520_mbox_chan_id_to_mapbit(cp); > + th1520_mbox_rmw(priv, TH_1520_MBOX_MASK, BIT(mask_bit), 0); > + > + if (cp->type == TH_1520_MBOX_TYPE_DB) > + /* tx doorbell doesn't have ACK, rx doorbell requires isr */ > + tasklet_init(&cp->txdb_tasklet, th1520_mbox_txdb_tasklet, > + (unsigned long)cp); > + > + ret = request_irq(priv->irq, th1520_mbox_isr, > + IRQF_SHARED | IRQF_NO_SUSPEND, cp->irq_desc, chan); Mixing devm- and non-devm with shared interrupts is error-prone (or even discouraged). Your code looks here correct, but probably this deserves a comment that you investigated the path and it is not possible to trigger interrupt from another device while device is unbound. > + if (ret) { > + dev_err(priv->dev, "Unable to acquire IRQ %d\n", priv->irq); > + return ret; > + } > + > + return 0; > +} > + > +static void th1520_mbox_shutdown(struct mbox_chan *chan) > +{ > + struct th1520_mbox_priv *priv = to_th1520_mbox_priv(chan->mbox); > + struct th1520_mbox_con_priv *cp = chan->con_priv; > + int mask_bit; > + > + /* clear the chan mask */ > + mask_bit = th1520_mbox_chan_id_to_mapbit(cp); > + th1520_mbox_rmw(priv, TH_1520_MBOX_MASK, 0, BIT(mask_bit)); > + > + free_irq(priv->irq, chan); Odd order. This should be reverse order from startup. Why is it not? > +} > + > +static const struct mbox_chan_ops th1520_mbox_ops = { > + .send_data = th1520_mbox_send_data, > + .startup = th1520_mbox_startup, > + .shutdown = th1520_mbox_shutdown, > +}; > + > +static int th1520_mbox_init_generic(struct th1520_mbox_priv *priv) > +{ > +#ifdef CONFIG_PM_SLEEP > + priv->ctx = devm_kzalloc(priv->dev, sizeof(*priv->ctx), GFP_KERNEL); > + if (!priv->ctx) > + return -ENOMEM; > +#endif > + /* Set default configuration */ > + th1520_mbox_write(priv, 0xff, TH_1520_MBOX_CLR); > + th1520_mbox_write(priv, 0x0, TH_1520_MBOX_MASK); > + return 0; > +} > + > +static struct mbox_chan *th1520_mbox_xlate(struct mbox_controller *mbox, > + const struct of_phandle_args *sp) > +{ > + struct th1520_mbox_priv *priv = to_th1520_mbox_priv(mbox); > + struct th1520_mbox_con_priv *cp; > + u32 chan, type; > + > + if (sp->args_count != 2) { > + dev_err(mbox->dev, "Invalid argument count %d\n", > + sp->args_count); > + return ERR_PTR(-EINVAL); > + } > + > + chan = sp->args[0]; /* comm remote channel */ > + type = sp->args[1]; /* comm channel type */ > + > + if (chan >= mbox->num_chans) { > + dev_err(mbox->dev, "Not supported channel number: %d\n", chan); > + return ERR_PTR(-EINVAL); > + } > + > + if (chan == priv->cur_icu_cpu_id) { > + dev_err(mbox->dev, "Cannot communicate with yourself\n"); > + return ERR_PTR(-EINVAL); > + } > + > + if (type > TH_1520_MBOX_TYPE_DB) { > + dev_err(mbox->dev, "Not supported the type for channel[%d]\n", > + chan); > + return ERR_PTR(-EINVAL); > + } > + > + cp = mbox->chans[chan].con_priv; > + cp->type = type; > + > + return &mbox->chans[chan]; > +} > + > +static int th1520_mbox_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct th1520_mbox_priv *priv; > + struct resource *res; > + unsigned int remote_idx = 0; > + unsigned int i; > + int ret; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + if (of_property_read_u32(np, "thead,icu-cpu-id", &priv->cur_icu_cpu_id)) { > + dev_err(dev, "thead,icu-cpu-id is missing\n"); > + return -EINVAL; > + } > + > + if (priv->cur_icu_cpu_id != TH_1520_MBOX_ICU_CPU0 && > + priv->cur_icu_cpu_id != TH_1520_MBOX_ICU_CPU3) { > + dev_err(dev, "thead,icu-cpu-id is invalid\n"); > + return -EINVAL; > + } > + > + priv->dev = dev; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "local"); > + priv->local_icu[TH_1520_MBOX_ICU_CPU0] = devm_ioremap_resource(dev, res); Use proper wrapper over these two. > + if (IS_ERR(priv->local_icu[TH_1520_MBOX_ICU_CPU0])) > + return PTR_ERR(priv->local_icu[TH_1520_MBOX_ICU_CPU0]); > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "remote-icu0"); > + priv->remote_icu[0] = devm_ioremap_resource(dev, res); > + if (IS_ERR(priv->remote_icu[0])) > + return PTR_ERR(priv->remote_icu[0]); > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "remote-icu1"); > + priv->remote_icu[1] = devm_ioremap_resource(dev, res); > + if (IS_ERR(priv->remote_icu[1])) > + return PTR_ERR(priv->remote_icu[1]); > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "remote-icu2"); > + priv->remote_icu[2] = devm_ioremap_resource(dev, res); > + if (IS_ERR(priv->remote_icu[2])) > + return PTR_ERR(priv->remote_icu[2]); Best regards, Krzysztof
