On 12/09/2024 19:00, Valentina Fernandez wrote:
> Add a mailbox controller driver for the Microchip Inter-processor
> Communication (IPC), which is used to send and receive data between
> processors.
>
> The driver uses the RISC-V Supervisor Binary Interface (SBI) to
> communicate with software running in machine mode (M-mode) to access
> the IPC hardware block.
>
...
> +
> +static struct microchip_ipc *to_mchp_ipc_mbox(struct mbox_controller *mbox)
> +{
> + return container_of(mbox, struct microchip_ipc, controller);
> +}
> +
You need kerneldoc for exported functions.
> +u32 mchp_ipc_get_chan_id(struct mbox_chan *chan)
> +{
> + struct ipc_chan_info *chan_info = (struct ipc_chan_info *)chan->con_priv;
> +
> + return chan_info->id;
> +}
> +EXPORT_SYMBOL(mchp_ipc_get_chan_id);
EXPORT_SYMBOL_GPL
> +
...
> +static int mchp_ipc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mchp_ipc_probe ipc_info;
> + struct microchip_ipc *ipc;
> + struct ipc_chan_info *priv;
> + bool irq_avail = false;
> + int ret;
> + u32 chan_id;
> +
> + ret = sbi_probe_extension(SBI_EXT_MICROCHIP_TECHNOLOGY);
> + if (ret <= 0)
> + return dev_err_probe(dev, ret, "Microchip SBI extension not detected\n");
> +
> + ipc = devm_kzalloc(dev, sizeof(*ipc), GFP_KERNEL);
> + if (!ipc)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, ipc);
> +
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IPC_DMA_BIT_MASK));
> + if (ret)
> + return dev_err_probe(dev, ret, "dma_set_mask_and_coherent failed\n");
> +
> + ipc->buf_base = dmam_alloc_coherent(dev, sizeof(u32), &ipc->dma_addr, GFP_KERNEL);
> +
Drop blank line.
> + if (!ipc->buf_base)
> + return -ENOMEM;
> +
> + ret = mchp_ipc_sbi_send(SBI_EXT_IPC_PROBE, ipc->dma_addr);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "could not probe IPC SBI service\n");
> +
> + memcpy(&ipc_info, ipc->buf_base, sizeof(struct mchp_ipc_probe));
> + ipc->num_channels = ipc_info.num_channels;
> + ipc->hw_type = ipc_info.hw_type;
> +
> + ipc->chans = devm_kcalloc(dev, ipc->num_channels, sizeof(*ipc->chans), GFP_KERNEL);
> + if (!ipc->chans)
> + return -ENOMEM;
> +
> + ipc->dev = dev;
> + ipc->controller.txdone_irq = true;
> + ipc->controller.dev = ipc->dev;
> + ipc->controller.ops = &mchp_ipc_ops;
> + ipc->controller.chans = ipc->chans;
> + ipc->controller.num_chans = ipc->num_channels;
> + ipc->controller.of_xlate = mchp_ipc_mbox_xlate;
> +
> + for (chan_id = 0; chan_id < ipc->num_channels; chan_id++) {
> + priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ipc->chans[chan_id].con_priv = priv;
> + priv->id = chan_id;
> + }
> +
> + if (ipc->hw_type == MIV_IHC) {
> + ipc->cluster_cfg = devm_kcalloc(dev, num_online_cpus(),
> + sizeof(struct mchp_ipc_cluster_cfg),
> + GFP_KERNEL);
> + if (!ipc->cluster_cfg)
> + return -ENOMEM;
> +
> + if (mchp_ipc_get_cluster_aggr_irq(ipc))
> + irq_avail = true;
> + }
> +
> + if (!irq_avail)
> + return dev_err_probe(dev, -ENODEV, "missing interrupt property\n");
> +
> + ret = devm_mbox_controller_register(dev, &ipc->controller);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Inter-Processor communication (IPC) registration failed\n");
Fix alignment.
> +
> + return 0;
> +}
> +
> +MODULE_DEVICE_TABLE(of, mchp_ipc_of_match);
This is ALWAYS next to the definition.
> +
> +static struct platform_driver mchp_ipc_driver = {
> + .driver = {
> + .name = "microchip_ipc",
> + .of_match_table = of_match_ptr(mchp_ipc_of_match),
Drop of_match_ptr. You have warnings here.
> + },
> + .probe = mchp_ipc_probe,
> +};
> +
> +module_platform_driver(mchp_ipc_driver);
Best regards,
Krzysztof
