[PATCH 2/2] reset: rockchip: implement rk3588 lookup table

[Kever Yang]

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-0c7344aa0a00@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