Re: [PATCH v2 1/2] soc: samsung: exynos-pmu: Add regmap support for SoCs that protect PMU regs

[Guenter Roeck <linux@xxxxxxxxxxxx>]

On 1/29/24 13:19, Peter Griffin wrote:
> Some Exynos based SoCs like Tensor gs101 protect the PMU registers for
> security hardening reasons so that they are only accessible in el3 via an
> SMC call.
>
> As most Exynos drivers that need to write PMU registers currently obtain a
> regmap via syscon (phys, pinctrl, watchdog). Support for the above usecase
> is implemented in this driver using a custom regmap similar to syscon to
> handle the SMC call. Platforms that don't secure PMU registers, get a mmio
> regmap like before. As regmaps abstract out the underlying register access
> changes to the leaf drivers are minimal.
>
> A new API exynos_get_pmu_regmap_by_phandle() is provided for leaf drivers
> that currently use syscon_regmap_lookup_by_phandle(). This also handles
> deferred probing.
>
> Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx>
> ---
[ ... ]

> +/**
> + * exynos_get_pmu_regmap
> + * Find the pmureg previously configured in probe() and return regmap property.
> + * Return: regmap if found or error if not found.
> + */
>   struct regmap *exynos_get_pmu_regmap(void)
>   {
>   	struct device_node *np = of_find_matching_node(NULL,
>   						      exynos_pmu_of_device_ids);
>   	if (np)
> -		return syscon_node_to_regmap(np);
> +		return exynos_get_pmu_regmap_by_phandle(np, NULL);
>   	return ERR_PTR(-ENODEV);
>   }
>   EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap);
>   
> +/**
> + * exynos_get_pmu_regmap_by_phandle
> + * Find the pmureg previously configured in probe() and return regmap property.
> + * Return: regmap if found or error if not found.
> + *
> + * @np: Pointer to device's Device Tree node
> + * @property: Device Tree property name which references the pmu
> + */
> +struct regmap *exynos_get_pmu_regmap_by_phandle(struct device_node *np,
> +						const char *property)
> +{
> +	struct device *dev;
> +	struct exynos_pmu_context *ctx;
> +	struct device_node *pmu_np;
> +
> +	if (property)
> +		pmu_np = of_parse_phandle(np, property, 0);
> +	else
> +		pmu_np = np;
> +
> +	if (!pmu_np)
> +		return ERR_PTR(-ENODEV);
> +
> +	dev = driver_find_device_by_of_node(&exynos_pmu_driver.driver,
> +					    (void *)pmu_np);
> +	of_node_put(pmu_np);
> +	if (!dev)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	ctx = dev_get_drvdata(dev);
> +
> +	return ctx->pmureg;
> +}
> +EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap_by_phandle);
> +

I think there should be a detailed comment explaining why the complexity
is necessary instead of just returning pmu_context->pmureg.

Thanks,
Guenter