Hi Kever,
On Fri, Apr 14, 2023 at 12:03:00PM +0300, Eugen Hristev wrote:
> On 4/14/23 10:02, Kever Yang wrote:
> > On 2023/4/13 19:36, Eugen Hristev wrote:
> > > The current DT bindings for the rk3588 clock use a different ID than the
> > > one that is supposed to be written to the hardware registers.
> > > Thus, we cannot use directly the id provided in the phandle, but rather
> > > use a lookup table to correctly setup the hardware.
> > >
> > > This approach has been implemented already in Linux, by commit :
> > > f1c506d152ff ("clk: rockchip: add clock controller for the RK3588")
> > >
> > > Hence, implement a similar approach using the lookup table, and adapt
> > > the existing reset driver to work with SoCs using lookup table.
> > > The file rst-rk3588.c has been copied as much as possible from Linux.
> >
> > I didn't follow the kernel closely, I'm not sure if there have any
> > discussion on this,
> >
> > but I think we can reuse the generic reset driver
> > "drivers/reset/reset-rockchip.c"
> >
> > as we have done for all other SoCs, but not add a new file for rk3588
> > and more for
> >
> > other SoCs. What we need to do is update the ID with encode version in
> > the header
> >
> > file like rockchip vendorkernel/U-Boot.
>
> This would not work, because in vendor U-boot we have different numbers for
> the same defines.
>
> E.g.:
>
> Vendor U-boot:
> #define SRST_PCIE4_POWER_UP 529
>
> While in upstream Linux/Uboot:
> #define SRST_PCIE4_POWER_UP 298
>
> So to be able to keep this file in sync with Linux :
> https://elixir.bootlin.com/u-boot/latest/source/include/dt-bindings/clock/rockchip,rk3588-cru.h
>
> ... we need to have a translation table. We cannot update the id
> in the header above. This would break the ABI and compatibility
> with Linux.You can follow the discussion that lead to the difference between upstream and vendor reset handling in this thread: https://lore.kernel.org/all/97bb42c0-725f-3611-ff3f-0c7344aa0...@linaro.org/ -- Sebastian
signature.asc
Description: PGP signature
