[U-Boot] [PATCH v2] rockchip: i2c: enable new I2C controller for rk3066 and rk3188
rk3066 and rk3188 has two I2C controller implementations. Current I2C driver wan't work with legacy implementation. Switching between controllers is performed using a bit inside GFR_SOC_CON1 register. The bit setting is performed by pinctrl driver. The patch ask pinctrl to do settings.
Signed-off-by: Alexander Kochetkov al.kochet@gmail.com --- drivers/i2c/rk_i2c.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c index 332280c..ce6b441 100644 --- a/drivers/i2c/rk_i2c.c +++ b/drivers/i2c/rk_i2c.c @@ -34,6 +34,18 @@ struct rk_i2c { unsigned int speed; };
+enum { + RK_I2C_LEGACY, + RK_I2C_NEW, +}; + +/** + * @controller_type: i2c controller type + */ +struct rk_i2c_soc_data { + int controller_type; +}; + static inline void rk_i2c_get_div(int div, int *divh, int *divl) { *divl = div / 2; @@ -381,9 +393,38 @@ static int rockchip_i2c_ofdata_to_platdata(struct udevice *bus) static int rockchip_i2c_probe(struct udevice *bus) { struct rk_i2c *priv = dev_get_priv(bus); + struct rk_i2c_soc_data *soc_data; + struct udevice *pinctrl; + int bus_nr; + int ret;
priv->regs = dev_read_addr_ptr(bus);
+ soc_data = (struct rk_i2c_soc_data*)dev_get_driver_data(bus); + + if (soc_data->controller_type == RK_I2C_LEGACY) { + ret = dev_read_alias_seq(bus, &bus_nr); + if (ret < 0) { + debug("%s: Could not get alias for %s: %d\n", + __func__, bus->name, ret); + return ret; + } + + ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl); + if (ret) { + debug("%s: Cannot find pinctrl device\n", __func__); + return ret; + } + + /* pinctrl will switch I2C to new type */ + ret = pinctrl_request_noflags(pinctrl, PERIPH_ID_I2C0 + bus_nr); + if (ret) { + debug("%s: Failed to switch I2C to new type %s: %d\n", + __func__, bus->name, ret); + return ret; + } + } + return 0; }
@@ -392,12 +433,51 @@ static const struct dm_i2c_ops rockchip_i2c_ops = { .set_bus_speed = rockchip_i2c_set_bus_speed, };
+static const struct rk_i2c_soc_data rk3066_soc_data = { + .controller_type = RK_I2C_LEGACY, +}; + +static const struct rk_i2c_soc_data rk3188_soc_data = { + .controller_type = RK_I2C_LEGACY, +}; + +static const struct rk_i2c_soc_data rk3228_soc_data = { + .controller_type = RK_I2C_NEW, +}; + +static const struct rk_i2c_soc_data rk3288_soc_data = { + .controller_type = RK_I2C_NEW, +}; + +static const struct rk_i2c_soc_data rk3328_soc_data = { + .controller_type = RK_I2C_NEW, +}; + +static const struct rk_i2c_soc_data rk3399_soc_data = { + .controller_type = RK_I2C_NEW, +}; + static const struct udevice_id rockchip_i2c_ids[] = { - { .compatible = "rockchip,rk3066-i2c" }, - { .compatible = "rockchip,rk3188-i2c" }, - { .compatible = "rockchip,rk3288-i2c" }, - { .compatible = "rockchip,rk3328-i2c" }, - { .compatible = "rockchip,rk3399-i2c" }, + { + .compatible = "rockchip,rk3066-i2c", + .data = (ulong)&rk3066_soc_data, + }, + { + .compatible = "rockchip,rk3188-i2c", + .data = (ulong)&rk3188_soc_data, + }, + { + .compatible = "rockchip,rk3288-i2c", + .data = (ulong)&rk3288_soc_data, + }, + { + .compatible = "rockchip,rk3328-i2c", + .data = (ulong)&rk3328_soc_data, + }, + { + .compatible = "rockchip,rk3399-i2c", + .data = (ulong)&rk3399_soc_data, + }, { } };
Alexander,
Sorry for being a bother. See below for two comments.
Cheers, Phil.
On 26 Feb 2018, at 18:42, Alexander Kochetkov al.kochet@gmail.com wrote:
rk3066 and rk3188 has two I2C controller implementations. Current I2C driver wan't work with legacy implementation. Switching between controllers is performed using a bit inside GFR_SOC_CON1 register. The bit setting is performed by pinctrl driver. The patch ask pinctrl to do settings.
Signed-off-by: Alexander Kochetkov al.kochet@gmail.com
drivers/i2c/rk_i2c.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c index 332280c..ce6b441 100644 --- a/drivers/i2c/rk_i2c.c +++ b/drivers/i2c/rk_i2c.c @@ -34,6 +34,18 @@ struct rk_i2c { unsigned int speed; };
+enum {
- RK_I2C_LEGACY,
- RK_I2C_NEW,
+};
+/**
- @controller_type: i2c controller type
- */
+struct rk_i2c_soc_data {
- int controller_type;
+};
static inline void rk_i2c_get_div(int div, int *divh, int *divl) { *divl = div / 2; @@ -381,9 +393,38 @@ static int rockchip_i2c_ofdata_to_platdata(struct udevice *bus) static int rockchip_i2c_probe(struct udevice *bus) { struct rk_i2c *priv = dev_get_priv(bus);
struct rk_i2c_soc_data *soc_data;
struct udevice *pinctrl;
int bus_nr;
int ret;
priv->regs = dev_read_addr_ptr(bus);
soc_data = (struct rk_i2c_soc_data*)dev_get_driver_data(bus);
if (soc_data->controller_type == RK_I2C_LEGACY) {
This should be safe both from legacy and new variants.
ret = dev_read_alias_seq(bus, &bus_nr);if (ret < 0) {debug("%s: Could not get alias for %s: %d\n",__func__, bus->name, ret);return ret;}ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl);if (ret) {debug("%s: Cannot find pinctrl device\n", __func__);return ret;}/* pinctrl will switch I2C to new type */ret = pinctrl_request_noflags(pinctrl, PERIPH_ID_I2C0 + bus_nr);if (ret) {debug("%s: Failed to switch I2C to new type %s: %d\n",__func__, bus->name, ret);return ret;}
I wonder if this is really necessary (or if there’s something going wrong in the device framework)… The way I always understood our device framework was that if there’s a pinctrl associated with the DTS node, then it should get processed automatically during probing.
See the following in drivers/core/device.c:
/* * Process pinctrl for everything except the root device, and * continue regardless of the result of pinctrl. Don't process pinctrl * settings for pinctrl devices since the device may not yet be * probed. */ if (dev->parent && device_get_uclass_id(dev) != UCLASS_PINCTRL) pinctrl_select_state(dev, "default");
- }
- return 0;
}
@@ -392,12 +433,51 @@ static const struct dm_i2c_ops rockchip_i2c_ops = { .set_bus_speed = rockchip_i2c_set_bus_speed, };
+static const struct rk_i2c_soc_data rk3066_soc_data = {
- .controller_type = RK_I2C_LEGACY,
+};
+static const struct rk_i2c_soc_data rk3188_soc_data = {
- .controller_type = RK_I2C_LEGACY,
+};
+static const struct rk_i2c_soc_data rk3228_soc_data = {
- .controller_type = RK_I2C_NEW,
+};
+static const struct rk_i2c_soc_data rk3288_soc_data = {
- .controller_type = RK_I2C_NEW,
+};
+static const struct rk_i2c_soc_data rk3328_soc_data = {
- .controller_type = RK_I2C_NEW,
+};
+static const struct rk_i2c_soc_data rk3399_soc_data = {
- .controller_type = RK_I2C_NEW,
+};
static const struct udevice_id rockchip_i2c_ids[] = {
- { .compatible = "rockchip,rk3066-i2c" },
- { .compatible = "rockchip,rk3188-i2c" },
- { .compatible = "rockchip,rk3288-i2c" },
- { .compatible = "rockchip,rk3328-i2c" },
- { .compatible = "rockchip,rk3399-i2c" },
- {
.compatible = "rockchip,rk3066-i2c",.data = (ulong)&rk3066_soc_data,- },
- {
.compatible = "rockchip,rk3188-i2c",.data = (ulong)&rk3188_soc_data,- },
- {
.compatible = "rockchip,rk3288-i2c",.data = (ulong)&rk3288_soc_data,- },
- {
.compatible = "rockchip,rk3328-i2c",.data = (ulong)&rk3328_soc_data,- },
- {
.compatible = "rockchip,rk3399-i2c",.data = (ulong)&rk3399_soc_data,- }, { }
};
-- 1.7.9.5
26 февр. 2018 г., в 23:26, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com написал(а):
I wonder if this is really necessary (or if there’s something going wrong in the device framework)… The way I always understood our device framework was that if there’s a pinctrl associated with the DTS node, then it should get processed automatically during probing.
RKI2C0_SEL, RKI2C1_SEL and so on bits get updated by pinctrl_rk3188_i2c_config() (drivers/pinctrl/rockchip/pinctrl_rk3188.c). The function is called by rk3188_pinctrl_request() (rk3188 pinmux request method implementation) and by rk3188_pinctrl_set_state_simple() (rk3188 pinmux set_state_simple implementation).
If CONFIG_PINCTRL_FULL is enabled, than pinctrl_select_state_full() will be used by framework code to setup pinmux. It read pin description from DT and setup pins using rk3188_pinctrl_set_state() (set_state implementation). rk3188_pinctrl_set_state() doesn't update soc_con1 register.
If CONFIG_PINCTRL_FULL is not enabled, then pinctrl_select_state_simple() will be used instead by framework code to setup pinmux. It do rk3188_pinctrl_set_state_simple() request to pinmux driver which in turn calls pinctrl_rk3188_i2c_config().
In my code I do call to pinctrl_rk3188_i2c_config() using rk3188_pinctrl_request().
Regards, Alexander.
rk3066 and rk3188 has two I2C controller implementations. Current I2C driver wan't work with legacy implementation. Switching between controllers is performed using a bit inside GFR_SOC_CON1 register. The bit setting is performed by pinctrl driver. The patch ask pinctrl to do settings.
Signed-off-by: Alexander Kochetkov al.kochet@gmail.com
drivers/i2c/rk_i2c.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 5 deletions(-)
Acked-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
rk3066 and rk3188 has two I2C controller implementations. Current I2C driver wan't work with legacy implementation. Switching between controllers is performed using a bit inside GFR_SOC_CON1 register. The bit setting is performed by pinctrl driver. The patch ask pinctrl to do settings.
Signed-off-by: Alexander Kochetkov al.kochet@gmail.com Acked-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/i2c/rk_i2c.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 5 deletions(-)
Applied to u-boot-rockchip, thanks!
participants (3)
-
Alexander Kochetkov -
Dr. Philipp Tomsich -
Philipp Tomsich