Re: [PATCH v2 4/6] pmdomain: renesas: Add R-Car X5H MDLC driver
From: Ulf Hansson
Date: Tue Jul 14 2026 - 08:20:31 EST
On Wed, Jul 8, 2026 at 12:15 PM Geert Uytterhoeven
<geert+renesas@xxxxxxxxx> wrote:
>
> Add a minimal Module Controller driver for the R-Car X5H (R8A78000) SoC.
> For now this just supports the always-on power domains, and dummy clocks
> and resets for the serial console (which is enabled by the boot loader).
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> Calling genpd_add_provider() instead of of_genpd_add_provider_onecell()
> would be sufficient, but the former is private.
Right. Callers that need their own xlate callback should just assign
it before calling of_genpd_add_provider_onecell(), as the below code
does.
>
> v2:
> - Spin off from "pmdomain: renesas: Add R-Car X5H MDLC SCMI remapping
> driver",
> - Add default support not using SCMI,
> - Drop all SCMI remapping support,
> - Document use of -1 as a sentinel,
> - Rename struct r8a78000_mdlc_info to mdlc_info,
> - Print HW IDs in hexadecimal,
> - Reduce log level for unsupported MDLC instances from warn to dbg,
> ---
> drivers/pmdomain/renesas/Kconfig | 4 +
> drivers/pmdomain/renesas/Makefile | 1 +
> drivers/pmdomain/renesas/r8a78000-mdlc.c | 329 +++++++++++++++++++++++
> drivers/soc/renesas/Kconfig | 1 +
> 4 files changed, 335 insertions(+)
> create mode 100644 drivers/pmdomain/renesas/r8a78000-mdlc.c
>
> diff --git a/drivers/pmdomain/renesas/Kconfig b/drivers/pmdomain/renesas/Kconfig
> index b507c3e0d723efc6..f2f52d3c29a083f1 100644
> --- a/drivers/pmdomain/renesas/Kconfig
> +++ b/drivers/pmdomain/renesas/Kconfig
> @@ -13,6 +13,10 @@ config SYSC_RMOBILE
> bool "System Controller support for R-Mobile" if COMPILE_TEST
>
> # SoC
> +config MDLC_R8A78000
> + bool "Module Controller support for R8A78000 (R-Car X5H)" if COMPILE_TEST
> + select RESET_CONTROLLER
> +
> config SYSC_R8A7742
> bool "System Controller support for R8A7742 (RZ/G1H)" if COMPILE_TEST
> select SYSC_RCAR
> diff --git a/drivers/pmdomain/renesas/Makefile b/drivers/pmdomain/renesas/Makefile
> index 0391e6e67440a786..17849aad37a5ac4f 100644
> --- a/drivers/pmdomain/renesas/Makefile
> +++ b/drivers/pmdomain/renesas/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> # SoC
> +obj-$(CONFIG_MDLC_R8A78000) += r8a78000-mdlc.o
> obj-$(CONFIG_SYSC_R8A7742) += r8a7742-sysc.o
> obj-$(CONFIG_SYSC_R8A7743) += r8a7743-sysc.o
> obj-$(CONFIG_SYSC_R8A7745) += r8a7745-sysc.o
> diff --git a/drivers/pmdomain/renesas/r8a78000-mdlc.c b/drivers/pmdomain/renesas/r8a78000-mdlc.c
> new file mode 100644
> index 0000000000000000..ed367e921a3341a7
> --- /dev/null
> +++ b/drivers/pmdomain/renesas/r8a78000-mdlc.c
> @@ -0,0 +1,329 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * R-Car X5H Module Controller
> + *
> + * Copyright (C) 2026 Glider bv
> + */
> +
> +#include <linux/dev_printk.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/power/renesas,r8a78000-mdlc.h>
> +
> +struct mod_map {
> + int hw_id; /* Hardware module ID or -1 sentinel */
> +};
> +
> +struct mdlc_info {
> + u32 base;
> + const struct mod_map *mod_map;
> +};
> +
> +/**
> + * struct r8a78000_mdlc_priv - Module Controller Private Data
> + *
> + * @link: Link into list of MDLC instances
> + * @genpd_data: PM domain provider data
> + * @rcdev: Reset controller entity
> + * @dev: MDLC device
> + * @np: Device node in DT representing the MDLC
> + * @mod_map: Mapping from hardware module IDs
> + */
> +struct r8a78000_mdlc_priv {
> + struct hlist_node link;
> + struct genpd_onecell_data genpd_data;
> + struct reset_controller_dev rcdev;
> + struct device *dev;
> + struct device_node *np;
> + const struct mod_map *mod_map;
> +};
> +
> +static struct generic_pm_domain *r8a78000_genpd_always_on;
> +static HLIST_HEAD(r8a78000_mdlc_list);
> +static DEFINE_MUTEX(r8a78000_mdlc_lock); /* protects the two above */
> +
> +static struct generic_pm_domain *r8a78000_genpd_xlate(
> + const struct of_phandle_args *spec, void *data)
> +{
> + struct r8a78000_mdlc_priv *priv = container_of(data,
> + struct r8a78000_mdlc_priv, genpd_data);
> + struct device *dev = priv->dev;
> + u32 id;
> +
> + if (spec->args_count != 2)
> + return ERR_PTR(-EINVAL);
> +
> + id = spec->args[0];
> +
> + if (id >= R8A78000_MDLC_PD_AON) {
> + dev_dbg(dev,
> + "Mapping HW power domain 0x%x to always-on domain\n",
> + id);
> + return r8a78000_genpd_always_on;
This looks odd, but perhaps it's just a temporary mapping that you
intend to change later, no?
> + }
> +
> + dev_err(dev, "Unknown power domain 0x%x\n", id);
> + return ERR_PTR(-ENOENT);
> +}
[...]
> +
> +static void r8a78000_genpd_del_provider(void *data)
> +{
> + of_genpd_del_provider(data);
> +}
> +
> +static int r8a78000_genpd_always_on_singleton(struct device *dev)
> +{
> + struct generic_pm_domain *genpd;
> + int ret;
> +
> + guard(mutex)(&r8a78000_mdlc_lock);
> +
> + if (r8a78000_genpd_always_on)
> + return 0;
> +
I guess the mutex is used to protect the global
r8a78000_genpd_always_on, but it looks like that isn't really needed
based upon how things are being called during the probe.
> + genpd = kzalloc_obj(*genpd);
> + if (!genpd)
> + return -ENOMEM;
> +
> + genpd->name = "always-on";
> + genpd->attach_dev = r8a78000_mdlc_attach_dev;
> +
> + ret = pm_genpd_init(genpd, &pm_domain_always_on_gov, false);
> + if (ret) {
> + kfree(genpd);
> + return dev_err_probe(dev, ret,
> + "Failed to create always-on domain\n");
> + }
> +
> + r8a78000_genpd_always_on = genpd;
> + return 0;
> +}
> +
> +static int r8a78000_mdlc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct r8a78000_mdlc_priv *priv;
> + const struct mdlc_info *info;
> + struct resource *res;
> + int ret;
> +
> + ret = r8a78000_genpd_always_on_singleton(dev);
> + if (ret)
> + return ret;
> +
> + info = of_device_get_match_data(dev);
> + if (!info)
> + return -ENODEV;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = dev;
> + priv->np = np;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + for (; info->base; info++) {
> + if (info->base == res->start)
> + break;
> + }
> +
> + if (!info->base) {
> + dev_dbg(dev, "Unsupported MDLC instance 0x%pa\n", &res->start);
> + return -ENODEV;
> + }
> +
> + priv->mod_map = info->mod_map;
> +
> + scoped_guard(mutex, &r8a78000_mdlc_lock) {
> + hlist_add_head(&priv->link, &r8a78000_mdlc_list);
> + }
> +
> + ret = devm_add_action_or_reset(dev, r8a78000_mdlc_unlink, priv);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to add action\n");
> +
> + /* Note that no actual domains are registered, just need translation */
> + priv->genpd_data.xlate = r8a78000_genpd_xlate;
> + ret = of_genpd_add_provider_onecell(np, &priv->genpd_data);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to register genpd provider\n");
> +
> + ret = devm_add_action_or_reset(dev, r8a78000_genpd_del_provider, np);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to add unregister action\n");
> +
> + priv->rcdev.ops = &r8a78000_mdlc_reset_ops;
> + priv->rcdev.of_node = np;
> + priv->rcdev.of_reset_n_cells = 1;
> + priv->rcdev.of_xlate = r8a78000_mdlc_reset_xlate;
> +
> + ret = devm_reset_controller_register(dev, &priv->rcdev);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to register reset controller\n");
> +
In some of the error paths above it looks like we end up leaking the
data allocated for r8a78000_genpd_always_on. Perhaps clean up that
somewhere here.
> + return 0;
> +}
[...]
In regards to the merge strategy, I intend to pick patch 2 (shared via
the immutable dt branch) and $subject patch. Please let me know if
there are any issues with that.
Kind regards
Uffe