[U-Boot] Rockchip GRF/PMUGRF/CRU reg access in U-Boot

Kever Yang kever.yang at rock-chips.com
Fri Oct 20 01:59:25 UTC 2017


Hi Simon and Philipp.

     I would like to include Heiko who is maintainer of upstream kernel 
Rockchip SoC for this topic.

For the driver related to grf, which including pinctrl, usb phy, gmac 
and many other modules, we would like to sync with kernel, so that we 
can re-use the dtsi from kernel. The use of grf register have many 
discuss on upstream kernel already, and all modules use grf with a &grf 
reference and offset in dts.

U-Boot coding style says "Use structures for I/O access", in my opinion, 
this is good for most case, but not for registers like Rockchip grf, 
because it's related for many modules, and it's different in different 
SoC, use "base addr + offset" for grf reg in dtsi and decode it in 
driver is much more reasonable for grf related driver setting. Just like 
the line size 80 is max, we follow the rule in most case, but in some 
case, more than 80 characters is better for read, then we use more than 
80 characters. We can merge pinctrl driver into one file like kernel if 
we can use this feature, and the same with sysreset, soft reset...We are 
not going to follow the dead rules for all cases, we have to use the one 
which is better for us.

To make it more clear, here is an example:

Looking at drivers/sysreset/sysreset_rk3xxx.c(there are 8 files and more 
to come), the source code are almost the same, the only difference is 
the reg address of glb_srst_snd_value and glb_srst_fst_value in cru,  we 
can very easy to decode this from dts in one common driver if we use 
'base + offset'. If we insist on using structure for this kind of reg 
access, to merge them into one common driver we have to including all 
different SoC cru definition and use different variable for different 
SoC, there would be many '#ifdef ... #else' in the file like this:

#ifdef CONFIG_ROCKCHIP_RK3036

#include <asm/arch/cru_rk3036.h>

#else ifdef CONFIG_ROCKCHIP_RK3188

#include <asm/arch/cru_rk3188.h>

#else ifdef CONFIG_ROCKCHIP_RK322x

#include <asm/arch/cru_rk322x.h>

#else ifdef CONFIG_ROCKCHIP_RK3288

#include <asm/arch/cru_rk3288.h>

#else ifdef CONFIG_ROCKCHIP_RK3328

#include <asm/arch/cru_rk3288.h>

#else ifdef CONFIG_ROCKCHIP_RK3368

#include <asm/arch/cru_rk3368.h>

...
#endif

and also this is need when we define and use the cru variant.


I would like to use two dts decode instead like this for all SoCs:
rphy->grf_base = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
ofnode_read_u32(dev_ofnode(dev), "reg", &reg)
sysreset_reg = rphy->grf_base + reg;

and dts for different SoC like this:
         rockchip,grf = <&grf>;
         reg = <0x017c>;

We wish upstream U-Boot can accept this kind of coding, I believe when 
the coding style was made, it does not met the controller reg like 
Rockchip GRF.
We will follow the coding style requirement in all other normal 
controller like I2C/SPI/USB/SDMMC/DISPLAY and etc.

Thanks,
- Kever


More information about the U-Boot mailing list