Hello, Philipp!
What exactly are you trying to configure here?
Do you need to bring these out of reset, is this some IO config or are these clock gates? Note that if it’s any of these, then the respective drivers (i.e. reset, pinctrl, clock) should be modified instead of putting this here.
This is rki2c_sel bit used to switch I2C IP between different implementations. Here comment from TRM: "1: rki2c is used instead of old i2c». U-boot and kernel I2C drivers want work with legacy IP implementation.
The patch is backport from upstream linux and the code was located inside I2C driver code.
+static const struct rk_i2c_soc_data rk3066_soc_data = {
- .grf_offset = 0x154,
Please don’t use open-coded constants for something like this.
This is GRF_SON_CON1 register. A lot of rockchip drivers inside linux kernel use following technic to make drivers universal between different chips. Logically it is not far away from simple constant definition using define syntax. I don’t use that constant inside functions directly.
I tried to replace constant with following code:
#include <asm/arch/grf_rk3036.h> #include <asm/arch/grf_rk3188.h>
...
static const struct rk_i2c_soc_data rk3066_soc_data = { .grf_offset = offsetof(struct rk3036_grf, soc_con1), };
static const struct rk_i2c_soc_data rk3188_soc_data = { .grf_offset = offsetof(struct rk3188_grf, soc_con1), };
But the code wan’t compile, because grf_rk3036.h and grf_rk3188.h both define a lot of constants with same name and that produce a lot of build errors.
What about if I just add comment to grf_offset definintion something like that? That way it could be ease found using text search.
static const struct rk_i2c_soc_data rk3066_soc_data = { .grf_offset = 0x154, // offsetof(struct rk3036_grf, soc_con1), };
static const struct rk_i2c_soc_data rk3188_soc_data = { .grf_offset = 0x0a4, // offsetof(struct rk3188_grf, soc_con1), };
Another solution I can do is following. I declare RK3066_GRF_SOC_CON1 and RK3188_GFR_SOC_CON1 constants at top of i2c driver and use them.
static const struct rk_i2c_soc_data rk3066_soc_data = { .grf_offset = RK3066_GRF_SOC_CON1, };
static const struct rk_i2c_soc_data rk3188_soc_data = { .grf_offset = RK3188_GRF_SOC_CON1, };
What do you think? How to do better?
Regards, Alexander.