[PATCH v3 10/18] soc: imx: add i.MX8M blk-ctrl driver

[Philipp Zabel]

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 at pengutronix.de>
> ---
> 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 at pengutronix.de>
> + */
> +
> +#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, &regmap_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 at pengutronix.de>

regards
Philipp