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", ®)
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
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot
