Hi Sebastian and Eugen,

    Thanks for your information.

On 2023/4/14 21:15, Sebastian Reichel wrote:
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.
Yes, U-Boot need to use the same header file with Linux.

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/

I can understand why  need this header and new reset driver changes now.

I think rockchip would like to make it easy to use like legacy SoCs, and the ID also need to be unique/correct/easy to maintain,

so a little trick in the ID define and with a tools help to maintain will be a good solution, SDK developer only use the defined MACROs

like legacy SoCs, and only one rockchip-reset driver is needed.

Now we need add new reset drivers for each SoCs.


Thanks,

- Kever


-- Sebastian

Reply via email to