[U-Boot] [PATCH] rockchip: i2c: enable I2C inside GRF for rk3066 and rk3188

[Alexander Kochetkov]

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.