Hi Heiko, On Friday, 2 August 2024 10:34:07 EDT Heiko Stübner wrote: > Hi Detlev, > > Am Freitag, 2. August 2024, 16:12:49 CEST schrieb Detlev Casanova: > > From: Elaine Zhang <zhangqing@xxxxxxxxxxxxxx> > > > > Add the dt-bindings header for the rk3576, that gets shared between > > the clock controller and the clock references in the dts. > > > > Signed-off-by: Elaine Zhang <zhangqing@xxxxxxxxxxxxxx> > > Signed-off-by: Sugar Zhang <sugar.zhang@xxxxxxxxxxxxxx> > > Signed-off-by: Detlev Casanova <detlev.casanova@xxxxxxxxxxxxx> > > --- > > > > .../dt-bindings/clock/rockchip,rk3576-cru.h | 1149 +++++++++++++++++ > > 1 file changed, 1149 insertions(+) > > create mode 100644 include/dt-bindings/clock/rockchip,rk3576-cru.h > > > > diff --git a/include/dt-bindings/clock/rockchip,rk3576-cru.h > > b/include/dt-bindings/clock/rockchip,rk3576-cru.h new file mode 100644 > > index 0000000000000..19d25f082dc57 > > --- /dev/null > > +++ b/include/dt-bindings/clock/rockchip,rk3576-cru.h > > @@ -0,0 +1,1149 @@ > > > > +#define CLK_NR_CLKS (ACLK_KLAD + 1) > > this needs to go please. Take a look at how Sebastian got rid of needed > that max-constant for rk3588. Oh indeed, that looks better, I'll port it to using the same functions as in rk3588. I'll also separate clocks and resets as done in rk3588 and improve the listing of those resets as in rst-rk3588.c > [...] > > > +#define SRST_H_VEPU1 1267 > > +#define SRST_A_VEPU1 1268 > > +#define SRST_VEPU1_CORE 1269 > > + > > +/********Name=PHPPHYSOFTRST_CON00,Offset=0x8A00********/ > > +#define SRST_P_PHPPHY_CRU 131073 > > +#define SRST_P_APB2ASB_SLV_CHIP_TOP 131075 > > +#define SRST_P_PCIE2_COMBOPHY0 131077 > > +#define SRST_P_PCIE2_COMBOPHY0_GRF 131078 > > +#define SRST_P_PCIE2_COMBOPHY1 131079 > > +#define SRST_P_PCIE2_COMBOPHY1_GRF 131080 > > this seems to lump together different components and with that creates > these gaps. I.e. I really don't think the phpphy in these registers is part > of the core CRU. > > That huge memory length of 0x5c000 in your dt-binding is also a good > indicator that this needs to have more separation and not span multiple > devices. It is not really clear if those are different devices, but they can possibly be seen as different instances of the same device. I'll just remove those extra resets for now and add them with the correct device when support is implemented. > > +/********Name=PHPPHYSOFTRST_CON01,Offset=0x8A04********/ > > +#define SRST_PCIE0_PIPE_PHY 131093 > > +#define SRST_PCIE1_PIPE_PHY 131096 > > + > > +/********Name=SECURENSSOFTRST_CON00,Offset=0x10A00********/ > > +#define SRST_H_CRYPTO_NS 262147 > > +#define SRST_H_TRNG_NS 262148 > > +#define SRST_P_OTPC_NS 262152 > > +#define SRST_OTPC_NS 262153 > > + > > +/********Name=PMU1SOFTRST_CON00,Offset=0x20A00********/ > > +#define SRST_P_HDPTX_GRF 524288 > > same here, that is also most likely not part of the CRU but a different > block. Other socs already implement separate clock controllers for > different parts, so please take a look there. Let's add those resets when the device they are linked to is actually supported then. > Thanks > Heiko Regards, Detlev.
