Hi Lucas,
On Mon, 2021-09-06 at 20:43 +0200, Lucas Stach wrote:
> This adds a driver for the blk-ctrl blocks found in the i.MX8M* line of
> SoCs. The blk-ctrl is a top-level peripheral located in the various *MIX
> power domains and interacts with the GPC power controller to provide the
> peripherals in the power domain access to the NoC and ensures that those
> peripherals are properly reset when their respective power domain is
> brought back to life.
>
> Software needs to do different things to make the bus handshake happen
> after the the GPC *MIX domain is power up and before it is powered down.
> As the requirements are quite different between the various blk-ctrls
> there is a callback function provided to hook in the proper sequence.
>
> The peripheral domains are quite uniform, they handle the soft clock
> enables and resets in the blk-ctrl address space and sequencing with the
> upstream GPC power domains.
>
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> ---
> This commit includes the full code to drive the VPUMIX domain on the
> i.MX8MM, as the skeleton driver would probably be harder to review
> without the context provided by one blk-ctrl implementation. Other
> blk-ctrl implementations will follow, based on this overall structure.
> ---
> drivers/soc/imx/Makefile | 1 +
> drivers/soc/imx/imx8m-blk-ctrl.c | 455 +++++++++++++++++++++++++++++++
> 2 files changed, 456 insertions(+)
> create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c
>
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> index 078dc918f4f3..8a707077914c 100644
> --- a/drivers/soc/imx/Makefile
> +++ b/drivers/soc/imx/Makefile
> @@ -5,3 +5,4 @@ endif
> obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
> obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
> obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> +obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> new file mode 100644
> index 000000000000..3dd17b903636
> --- /dev/null
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2021 Pengutronix, Lucas Stach <kernel@xxxxxxxxxxxxxx>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +
> +#include <dt-bindings/power/imx8mm-power.h>
> +
> +#define BLK_SFT_RSTN 0x0
> +#define BLK_CLK_EN 0x4
> +
> +struct imx8m_blk_ctrl_domain;
> +
> +struct imx8m_blk_ctrl {
> + struct device *dev;
> + struct notifier_block power_nb;
> + struct device *bus_power_dev;
> + struct regmap *regmap;
> + struct imx8m_blk_ctrl_domain *domains;
> + struct genpd_onecell_data onecell_data;
> +};
> +
> +struct imx8m_blk_ctrl_domain_data {
> + const char *name;
> + const char **clk_names;
const char * const *clk_names;
even?
> + int num_clks;
> + const char *gpc_name;
> + u32 rst_mask;
> + u32 clk_mask;
> +};
> +
[...]
> +static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
> +{
[...]
> + regmap_config.max_register = bc_data->max_reg;
> + bc->regmap = devm_regmap_init_mmio(dev, base, ®map_config);
> + if (IS_ERR(bc->regmap))
> + return dev_err_probe(dev, PTR_ERR(bc->regmap),
> + "failed to init regmap \n");
^
Superfluous whitespace.
> + bc->domains = devm_kcalloc(dev, bc_data->num_domains,
> + sizeof(struct imx8m_blk_ctrl_domain),
> + GFP_KERNEL);
^
Misalignment (also superfluous whitespace).
[...]
> +static int imx8mm_vpu_power_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
> + power_nb);
> +
> + if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
> + return NOTIFY_OK;
> +
> + /*
> + * The ADB in the VPUMIX domain has no separate reset and clock
> + * enable bits, but is ungated together with the VPU clocks. To
> + * allow the handshake with the GPC to progress we put the VPUs
> + * in reset and ungate the clocks.
> + */
> + regmap_clear_bits(bc->regmap, BLK_SFT_RSTN,
> + BIT(0) | BIT(1) | BIT(2));
> + regmap_set_bits(bc->regmap, BLK_CLK_EN,
> + BIT(0) | BIT(1) | BIT(2));
These can be put on a single line each.
Otherwise,
Reviewed-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
regards
Philipp
