Re: [PATCH 2/2] pmdomain: sunxi: Add support for A733 to Allwinner PCK600 driver

[Chen-Yu Tsai]

On Thu, Mar 5, 2026 at 9:15 AM Yuanshen Cao <alex.caoys@xxxxxxxxx> wrote:
>
> On Wed, Mar 04, 2026 at 10:33:23PM +0900, Chen-Yu Tsai wrote:
> > On Wed, Mar 4, 2026 at 12:44 PM Yuanshen Cao <alex.caoys@xxxxxxxxx> wrote:
> > >
> > > The Allwinner A733 PCK600, similar to A523 PCK600, is likely a
> > > customized version of ARM PCK-600 power controller. It shares
> > > the same BSP driver with A523. According to the BSP provided
> > > by Radxa, unlike A523, it doesn't require reset.
> > >
> > > Make reset optional in the sunxi pck600 driver and add support
> > > for A733.
> > >
> > > Signed-off-by: Yuanshen Cao <alex.caoys@xxxxxxxxx>
> > > ---
> > >  drivers/pmdomain/sunxi/sun55i-pck600.c | 53 ++++++++++++++++++++++++++++++----
> > >  1 file changed, 48 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/pmdomain/sunxi/sun55i-pck600.c b/drivers/pmdomain/sunxi/sun55i-pck600.c
> > > index c7ab51514531..8f9fdc3915bd 100644
> > > --- a/drivers/pmdomain/sunxi/sun55i-pck600.c
> > > +++ b/drivers/pmdomain/sunxi/sun55i-pck600.c
> > > @@ -52,6 +52,7 @@ struct sunxi_pck600_desc {
> > >         u32 logic_power_switch0_delay;
> > >         u32 logic_power_switch1_delay;
> > >         u32 off2on_delay;
> > > +       bool has_rst_clk;
> > >  };
> > >
> > >  struct sunxi_pck600_pd {
> > > @@ -151,9 +152,11 @@ static int sunxi_pck600_probe(struct platform_device *pdev)
> > >         if (IS_ERR(base))
> > >                 return PTR_ERR(base);
> > >
> > > -       rst = devm_reset_control_get_exclusive_released(dev, NULL);
> > > -       if (IS_ERR(rst))
> > > -               return dev_err_probe(dev, PTR_ERR(rst), "failed to get reset control\n");
> > > +       if (desc->has_rst_clk) {
> > > +               rst = devm_reset_control_get_exclusive_released(dev, NULL);
> > > +               if (IS_ERR(rst))
> > > +                       return dev_err_probe(dev, PTR_ERR(rst), "failed to get reset control\n");
> > > +       }
> > >
> > >         clk = devm_clk_get_enabled(dev, NULL);
> > >         if (IS_ERR(clk))
> > > @@ -193,7 +196,14 @@ static int sunxi_pck600_probe(struct platform_device *pdev)
> > >  }
> > >
> > >  static const char * const sun55i_a523_pck600_pd_names[] = {
> > > -       "VE", "GPU", "VI", "VO0", "VO1", "DE", "NAND", "PCIE"
> > > +       "VE",
> > > +       "GPU",
> > > +       "VI",
> > > +       "VO0",
> > > +       "VO1",
> > > +       "DE",
> > > +       "NAND",
> > > +       "PCIE",
> >
> > No need to change this part.
> >
>
> Yes, I agree. But since A733 has more pd_names, that line will be too
> long if we stick with one line. And we also used this format in
> sun20i-ppu. It might make more sense if we align with ppu. Please let
> me know what you think. Thanks!

I prefer fewer lines. And you can have more than one line even if you
put multiple names per line. The ppu driver is a different file, so I
didn't bother changing that one.

ChenYu

> > >  };
> > >
> > >  static const struct sunxi_pck600_desc sun55i_a523_pck600_desc = {
> > > @@ -206,7 +216,36 @@ static const struct sunxi_pck600_desc sun55i_a523_pck600_desc = {
> > >         .device_ctrl1_delay = 0xffff,
> > >         .logic_power_switch0_delay = 0x8080808,
> > >         .logic_power_switch1_delay = 0x808,
> > > -       .off2on_delay = 0x8
> > > +       .off2on_delay = 0x8,
> > > +       .has_rst_clk = true,
> > > +};
> > > +
> > > +static const char * const sun60i_a733_pck600_pd_names[] = {
> > > +       "VI",
> > > +       "DE_SYS",
> > > +       "VE_DEC",
> > > +       "VE_ENC",
> > > +       "NPU",
> > > +       "GPU_TOP",
> > > +       "GPU_CORE",
> > > +       "PCIE",
> > > +       "USB2",
> > > +       "VO",
> > > +       "VO1",
> > > +};
> > > +
> > > +static const struct sunxi_pck600_desc sun60i_a733_pck600_desc = {
> > > +       .pd_names = sun60i_a733_pck600_pd_names,
> > > +       .num_domains = ARRAY_SIZE(sun60i_a733_pck600_pd_names),
> > > +       .logic_power_switch0_delay_offset = 0xc00,
> > > +       .logic_power_switch1_delay_offset = 0xc04,
> > > +       .off2on_delay_offset = 0xc10,
> > > +       .device_ctrl0_delay = 0x1f1f1f,
> > > +       .device_ctrl1_delay = 0x1f1f,
> > > +       .logic_power_switch0_delay = 0x8080808,
> > > +       .logic_power_switch1_delay = 0x808,
> > > +       .off2on_delay = 0x8,
> > > +       .has_rst_clk = false,
> > >  };
> > >
> > >  static const struct of_device_id sunxi_pck600_of_match[] = {
> > > @@ -214,6 +253,10 @@ static const struct of_device_id sunxi_pck600_of_match[] = {
> > >                 .compatible     = "allwinner,sun55i-a523-pck-600",
> > >                 .data           = &sun55i_a523_pck600_desc,
> > >         },
> > > +       {
> > > +               .compatible     = "allwinner,sun60i-a733-pck-600",
> > > +               .data           = &sun60i_a733_pck600_desc,
> > > +       },
> > >         {}
> > >  };
> > >  MODULE_DEVICE_TABLE(of, sunxi_pck600_of_match);
> > >
> > > --
> > > 2.53.0
> > >
>