On 14/04/2026 18:15, Ben Levinsky wrote:
> Add an AMD MicroBlaze remoteproc driver.
>
> The driver parses the executable firmware memory window from
> the remoteproc device node's reg property, interprets that
> address and size in the MicroBlaze-local address space, and
> then uses standard devicetree address translation through the
> parent bus ranges property to obtain the corresponding
> Linux-visible system physical address.
>
> The resulting translated region is registered as the executable
> remoteproc carveout and coredump segment.
>
> The MicroBlaze is controlled through an active-low reset GPIO and kept in
> reset until firmware loading completes.
>
> The firmware-name property is optional, allowing firmware to be
> assigned later through the remoteproc framework.
>
Fix your commit msg so it uses sane style, not some indentation.
Look at how other commits are written, if you have doubts.
...
> +
> +static int mb_rproc_start(struct rproc *rproc)
> +{
> + struct mb_rproc *mb = rproc->priv;
> +
> + /* reset-gpios is declared active-low, so logical 0 releases reset */
If reset-gpios is declared active-high, then logical 0 also releases reset.
Drop comment, not correct.
> + gpiod_set_value_cansleep(mb->reset, 0);
> +
> + return 0;
> +}
> +
> +static int mb_rproc_stop(struct rproc *rproc)
> +{
> + struct mb_rproc *mb = rproc->priv;
> +
> + /* reset-gpios is declared active-low, so logical 1 asserts reset */
> + gpiod_set_value_cansleep(mb->reset, 1);
> +
> + return 0;
> +}
> +
> +static int mb_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> + int ret;
> +
> + ret = rproc_elf_load_rsc_table(rproc, fw);
> + if (ret == -EINVAL) {
> + dev_dbg(&rproc->dev, "no resource table found\n");
> + return 0;
> + }
> +
> + return ret;
> +}
> +
> +static const struct rproc_ops mb_rproc_ops = {
> + .prepare = mb_rproc_prepare,
> + .start = mb_rproc_start,
> + .stop = mb_rproc_stop,
> + .load = rproc_elf_load_segments,
> + .sanity_check = rproc_elf_sanity_check,
> + .get_boot_addr = rproc_elf_get_boot_addr,
> + .parse_fw = mb_rproc_parse_fw,
> +};
> +
> +static int mb_rproc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mb_rproc *mb;
> + struct rproc *rproc;
> + const char *fw_name = NULL;
> + int ret;
> +
> + ret = rproc_of_parse_firmware(dev, 0, &fw_name);
> + if (ret < 0 && ret != -EINVAL)
> + return dev_err_probe(dev, ret,
> + "failed to parse firmware-name
> property\n");
> +
> + rproc = devm_rproc_alloc(dev, dev_name(dev), &mb_rproc_ops, fw_name,
> + sizeof(*mb));
> + if (!rproc)
> + return -ENOMEM;
> +
> + mb = rproc->priv;
> + mb->dev = dev;
> +
> + /*
> + * Keep the MicroBlaze in reset until remoteproc has finished loading
> + * firmware into the executable memory window described by reg and
> + * translated through the parent bus ranges property.
> + */
> + mb->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(mb->reset))
> + return dev_err_probe(dev, PTR_ERR(mb->reset),
> + "failed to get reset gpio\n");
> +
> + rproc->auto_boot = false;
> +
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to set DMA mask\n");
> +
> + platform_set_drvdata(pdev, rproc);
> +
> + ret = devm_rproc_add(dev, rproc);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to register rproc\n");
> +
> + dev_dbg(dev, "MicroBlaze remoteproc registered\n");
Drop. This does not look like useful printk message. Drivers should be
silent on success:
https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/coding-style.rst#L913
https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/debugging/driver_development_debugging_guide.rst#L79
Core already gives way to see probe success.
Best regards,
Krzysztof
