Re: [PATCH 3/7] clk: sunxi-ng: de2: Export register regmap for DE33

Chen-Yu Tsai
2025-11-15T17:42:46Z

On Sat, Nov 15, 2025 at 10:14 PM Jernej Skrabec
<jernej.skrabec@gmail.com> wrote:
>
> DE33 clock pre-set plane mapping, which is not something that we want
> from clock driver. Export registers instead, so DRM driver can set them
> properly.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
> drivers/clk/sunxi-ng/ccu-sun8i-de2.c | 53 ++++++++++++++++++++++++++--
> 1 file changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
> index a6cd0f988859..2841ec922025 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
> @@ -6,9 +6,11 @@
> #include <linux/clk.h>
> #include <linux/clk-provider.h>
> #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/regmap.h>
> #include <linux/reset.h>
>
> #include "ccu_common.h"
> @@ -250,6 +252,41 @@ static const struct sunxi_ccu_desc sun50i_h616_de33_clk_desc = {
> .num_resets = ARRAY_SIZE(sun50i_h5_de2_resets),
> };
>
> +/*
> + * Add a regmap for the DE33 plane driver to access plane
> + * mapping registers.
> + * Only these registers are allowed to be written, to prevent
> + * overriding clock and reset configuration.
> + */
> +
> +#define SUN50I_DE33_CHN2CORE_REG 0x24
> +#define SUN50I_DE33_PORT02CHN_REG 0x28
> +#define SUN50I_DE33_PORT12CHN_REG 0x2c
> +
> +static bool sun8i_de2_ccu_regmap_accessible_reg(struct device *dev,
> + unsigned int reg)
> +{
> + switch (reg) {
> + case SUN50I_DE33_CHN2CORE_REG:
> + case SUN50I_DE33_PORT02CHN_REG:
> + case SUN50I_DE33_PORT12CHN_REG:
> + return true;
> + default:
> + return false;
> + }
> +}

Since the registers are contiguous, I think it makes a bit more sense
to use the .rd_table and .wr_table. A bonus is that the check can be
inlined in the core instead of calling a function pointer.

> +static const struct regmap_config sun8i_de2_ccu_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = 0xe0,

None of the registers past SUN50I_DE33_PORT12CHN_REG are accessible,
so we should probably just put that here.

> +
> + /* other devices have no business accessing other registers */
> + .readable_reg = sun8i_de2_ccu_regmap_accessible_reg,
> + .writeable_reg = sun8i_de2_ccu_regmap_accessible_reg,
> +};
> +
> static int sunxi_de2_clk_probe(struct platform_device *pdev)
> {
> struct clk *bus_clk, *mod_clk;
> @@ -303,13 +340,23 @@ static int sunxi_de2_clk_probe(struct platform_device *pdev)
> }
>
> /*
> - * The DE33 requires these additional (unknown) registers set
> + * The DE33 requires these additional plane mapping registers set
> * during initialisation.
> */
> if (of_device_is_compatible(pdev->dev.of_node,
> "allwinner,sun50i-h616-de33-clk")) {
> - writel(0, reg + 0x24);
> - writel(0x0000a980, reg + 0x28);
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init_mmio(&pdev->dev, reg,
> + &sun8i_de2_ccu_regmap_config);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + goto err_assert_reset;
> + }
> +
> + ret = of_syscon_register_regmap(pdev->dev.of_node, regmap);

dev_of_node(&pdev->dev) instead of directly accessing the member.
IIRC this is the new preferred style.

Thanks
ChenYu

> + if (ret)
> + goto err_assert_reset;
> }
>
> ret = devm_sunxi_ccu_probe(&pdev->dev, reg, ccu_desc);
> --
> 2.51.2
>
>