from:"Alexander Kochetkov"

[U-Boot] [PATCH] beagleboard: Remove side effects of i2c2 pullup resisters initialization code

2014-09-29 Thread Alexander Kochetkov

Fix typo of commit d4e53f063dd25e071444b87303573e7440deeb89.

i2c2 pullup resisters are controlled by bit 0 of CONTROL_PROG_IO1.
It's value after reset is 0x0011.

In order to clear bit 0, original code write 0xfffe to
CONTROL_PROG_IO1 and toggle almost all default values.

Original code affect following:
* disable i2c1 pullup resisters
* increase far end load setting for many modules
* setup invalid SC/LB combination

Signed-off-by: Alexander Kochetkov al.koc...@gmail.com
CC: Tom Rini tr...@ti.com
CC: Steve Kipisz s-kipi...@ti.com
---
 board/ti/beagle/beagle.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index 0674afd..94b99bf 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -317,9 +317,12 @@ int misc_init_r(void)
struct gpio *gpio6_base = (struct gpio *)OMAP34XX_GPIO6_BASE;
struct control_prog_io *prog_io_base = (struct control_prog_io 
*)OMAP34XX_CTRL_BASE;
bool generate_fake_mac = false;
+   u32 value;
 
/* Enable i2c2 pullup resisters */
-   writel(~(PRG_I2C2_PULLUPRESX), prog_io_base-io1);
+   value = readl(prog_io_base-io1);
+   value = ~(PRG_I2C2_PULLUPRESX);
+   writel(value, prog_io_base-io1);
 
switch (get_board_revision()) {
case REVISION_AXBX:
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mmc: fix off-by-one bug in mmc_startup_v4()

2018-02-21 Thread Alexander Kochetkov


> Do you mean SD-card or MMC-card? :)
> SD doesn't have EXT_CSD register.

I see now. MMC-card. So, send v2? or you can simple fix SD with MMC in commit 
msg.

> 
> - Removed Pantelis's mail account. Instead, add my account, plz.

I took it from here:
https://www.denx.de/wiki/U-Boot/Custodians

Regards,
Alexander.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] mmc: fix off-by-one bug in mmc_startup_v4()

2018-02-21 Thread Alexander Kochetkov


> 21 февр. 2018 г., в 9:37, Jaehoon Chung  написал(а):
> 
> I'm confusing about commit-msg. "SD-card with EXT_CSD_REV"?
> 
> Best Regards,
> Jaehoon Chung

I glad to write better, but don’t know. Would this one better?

In future, SD-cards with revision 9 (with REV value 9 inside EXT_CSD register)
will trigger off-by-one bug while accessing mmc_versions array.

Regards,
Alexander.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] mmc: fix eMMC v5.1 incorrect version detection

2018-02-20 Thread Alexander Kochetkov


> 20 февр. 2018 г., в 13:02, Jaehoon Chung  написал(а):
> 
>> Also the patch fix mmc_versions array bounds check. Value 8
>> produced out of array access.
> 
> It was already fixed. 
> 
> http://git.denx.de/?p=u-boot/u-boot-mmc.git;a=commit;h=ace1bed327411cf3cade45599864df2d461045a0
> 
> Best Regards,
> Jaehoon Chung

Thank you for pointing updated git tree.
I sent off-by-one bug fix from the patch as new patch.

Regards,
Alexander.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] mmc: fix eMMC v5.1 incorrect version detection

2018-02-20 Thread Alexander Kochetkov

eMMC cards v5.1 has value 8 inside EXT_CSD_REV register.
The patch make EXT_CSD_REV value 8 match v5.1

The is a hole inside version enumeration. EXT_CSD_REV value 4
doens't correspond to any valid eMMC version. So EXT_CSD_REV
value 4 assigned undefined version.

Also the patch fix mmc_versions array bounds check. Value 8
produced out of array access.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/mmc/mmc.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 255310a..c8c13bd 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1950,6 +1950,7 @@ static int mmc_startup_v4(struct mmc *mmc)
MMC_VERSION_4_1,
MMC_VERSION_4_2,
MMC_VERSION_4_3,
+   MMC_VERSION_UNKNOWN,
MMC_VERSION_4_41,
MMC_VERSION_4_5,
MMC_VERSION_5_0,
@@ -1973,7 +1974,8 @@ static int mmc_startup_v4(struct mmc *mmc)
return -ENOMEM;
memcpy(mmc->ext_csd, ext_csd, MMC_MAX_BLOCK_LEN);
 
-   if (ext_csd[EXT_CSD_REV] > ARRAY_SIZE(mmc_versions))
+   if (ext_csd[EXT_CSD_REV] >= ARRAY_SIZE(mmc_versions) ||
+   ext_csd[EXT_CSD_REV] == 4)
return -EINVAL;
 
mmc->version = mmc_versions[ext_csd[EXT_CSD_REV]];
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] mmc: fix off-by-one bug in mmc_startup_v4()

2018-02-20 Thread Alexander Kochetkov

SD-card with EXT_CSD_REV value 9 will trigger off-by-one
bug while accessing mmc_versions array. The patch fix that.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/mmc/mmc.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 99e2a75..3aa153a 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1974,7 +1974,7 @@ static int mmc_startup_v4(struct mmc *mmc)
return -ENOMEM;
memcpy(mmc->ext_csd, ext_csd, MMC_MAX_BLOCK_LEN);
 
-   if (ext_csd[EXT_CSD_REV] > ARRAY_SIZE(mmc_versions))
+   if (ext_csd[EXT_CSD_REV] >= ARRAY_SIZE(mmc_versions))
return -EINVAL;
 
mmc->version = mmc_versions[ext_csd[EXT_CSD_REV]];
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-02-20 Thread Alexander Kochetkov

In order to make I2C work on rk3066 and rk3188 boards GFR
must be updated.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/i2c/rk_i2c.c |   85 +++---
 1 file changed, 80 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c
index 332280c..3ec5474 100644
--- a/drivers/i2c/rk_i2c.c
+++ b/drivers/i2c/rk_i2c.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -34,6 +35,13 @@ struct rk_i2c {
unsigned int speed;
 };
 
+/**
+ * @grf_offset: offset inside the grf regmap for setting the i2c type
+ */
+struct rk_i2c_soc_data {
+   int grf_offset;
+};
+
 static inline void rk_i2c_get_div(int div, int *divh, int *divl)
 {
*divl = div / 2;
@@ -381,9 +389,37 @@ 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;
+   uint32_t value;
+   int bus_nr;
+   void *grf;
+   int ret;
 
priv->regs = dev_read_addr_ptr(bus);
 
+   soc_data = (struct rk_i2c_soc_data*)dev_get_driver_data(bus);
+
+   if (soc_data->grf_offset >= 0) {
+   grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
+   if (IS_ERR(grf)) {
+   ret = PTR_ERR(grf);
+   debug("%s: Could not get syscon for %s: %d\n",
+__func__, bus->name, ret);
+   return ret;
+   }
+
+   ret = dev_read_alias_seq(bus, _nr);
+   if (ret < 0) {
+   debug("%s: Could not get alias for %s: %d\n",
+__func__, bus->name, ret);
+   return ret;
+   }
+
+   /* 27+i: write mask, 11+i: value */
+   value = BIT(27 + bus_nr) | BIT(11 + bus_nr);
+   writel(value, grf + soc_data->grf_offset);
+   }
+
return 0;
 }
 
@@ -392,12 +428,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 = {
+   .grf_offset = 0x154,
+};
+
+static const struct rk_i2c_soc_data rk3188_soc_data = {
+   .grf_offset = 0x0a4,
+};
+
+static const struct rk_i2c_soc_data rk3228_soc_data = {
+   .grf_offset = -1,
+};
+
+static const struct rk_i2c_soc_data rk3288_soc_data = {
+   .grf_offset = -1,
+};
+
+static const struct rk_i2c_soc_data rk3328_soc_data = {
+   .grf_offset = -1,
+};
+
+static const struct rk_i2c_soc_data rk3399_soc_data = {
+   .grf_offset = -1,
+};
+
 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)_soc_data,
+   },
+   {
+   .compatible = "rockchip,rk3188-i2c",
+   .data = (ulong)_soc_data,
+   },
+   {
+   .compatible = "rockchip,rk3288-i2c",
+   .data = (ulong)_soc_data,
+   },
+   {
+   .compatible = "rockchip,rk3328-i2c",
+   .data = (ulong)_soc_data,
+   },
+   {
+   .compatible = "rockchip,rk3399-i2c",
+   .data = (ulong)_soc_data,
+   },
{ }
 };
 
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-02-26 Thread 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 
#include 

...

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.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2] rockchip: i2c: enable new I2C controller for rk3066 and rk3188

2018-02-26 Thread Alexander Kochetkov

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.koc...@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, _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, );
+   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)_soc_data,
+   },
+   {
+   .compatible = "rockchip,rk3188-i2c",
+   .data = (ulong)_soc_data,
+   },
+   {
+   .compatible = "rockchip,rk3288-i2c",
+   .data = (ulong)_soc_data,
+   },
+   {
+   .compatible = "rockchip,rk3328-i2c",
+   .data = (ulong)_soc_data,
+   },
+   {
+   .compatible = "rockchip,rk3399-i2c",
+   .data = (ulong)_soc_data,
+   },
{ }
 };
 
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2] rockchip: i2c: enable new I2C controller for rk3066 and rk3188

2018-02-26 Thread Alexander Kochetkov


> 26 февр. 2018 г., в 23:26, Dr. Philipp Tomsich 
>  написал(а):
> 
> 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.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 1/2] dm: i2c: dts: Add gpios and pinctrl device tree properties

2018-03-27 Thread Alexander Kochetkov

The commit describe usage of gpios and pinctrl device tree
properties in order to enable gpio-based software deblocking.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 doc/device-tree-bindings/i2c/i2c.txt |   13 +
 1 file changed, 13 insertions(+)

diff --git a/doc/device-tree-bindings/i2c/i2c.txt 
b/doc/device-tree-bindings/i2c/i2c.txt
index ea918dd..de818d4 100644
--- a/doc/device-tree-bindings/i2c/i2c.txt
+++ b/doc/device-tree-bindings/i2c/i2c.txt
@@ -12,6 +12,11 @@ property which allows the chip offset length to be selected.
 Optional properties:
 - u-boot,i2c-offset-len - length of chip offset in bytes. If omitted the
 default value of 1 is used.
+- gpios = , ;
+  pinctrl-names = "default", "gpio";
+  pinctrl-0 = <_xfer>;
+  pinctrl-1 = <_gpio>;
+Pin description for I2C bus software deblocking.
 
 
 Example
@@ -26,3 +31,11 @@ i2c4: i2c@12ca {
ec-interrupt = < 6 GPIO_ACTIVE_LOW>;
};
 };
+
+ {
+   pinctrl-names = "default", "gpio";
+   pinctrl-0 = <_xfer>;
+   pinctrl-1 = <_gpio>;
+   gpios = < 26 GPIO_ACTIVE_LOW>, /* SDA */
+   < 27 GPIO_ACTIVE_LOW>; /* SCL */
+};
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 0/2] dm: i2c: implement gpio-based I2C deblock

2018-03-27 Thread Alexander Kochetkov

Hello, Heiko! 

Here is try2 of my previous patch[1].

Done in v2:
- fix checkpatch warnings
- ENOSYS warning still present, but it was in original code also 
  and for me it looks like -ENOSYS must be returned in case of
  missed i2c_deblock_gpio() implementation.
WARNING: ENOSYS means 'invalid syscall nr' and nothing else
#138: FILE: drivers/i2c/i2c-uclass.c:552:
+   return -ENOSYS;
- fix travis build, results can be viewed here:
  https://travis-ci.org/akochetkov/u-boot/builds/358822512
- add property description to i2c.txt

I have question. I declared PIN_SDA, PIN_SCL, PIN_COUNT constants for
i2c-uclass.c. The same constants exist in the i2c-gpio.c. Should I place
them into i2c.h or may leave as is?

[1] https://lists.denx.de/pipermail/u-boot/2018-March/thread.html#321755

Alexander Kochetkov (2):
  dm: i2c: dts: Add gpios and pinctrl device tree properties
  dm: i2c: implement gpio-based I2C deblock

 doc/device-tree-bindings/i2c/i2c.txt |   13 
 drivers/i2c/i2c-uclass.c |  118 +++---
 2 files changed, 122 insertions(+), 9 deletions(-)

-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 2/2] dm: i2c: implement gpio-based I2C deblock

2018-03-27 Thread Alexander Kochetkov

The commit implement a gpio-based software deblocking. The code
extract I2C pins description from device tree, switch pins to GPIO
mode, toggle SCL until slave release SDA, send I2C stop and switch
I2C pins back to I2C mode.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/i2c/i2c-uclass.c |  118 ++
 1 file changed, 109 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index 920811a..4ac6ef8 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -11,9 +11,19 @@
 #include 
 #include 
 #include 
+#include 
+#ifdef CONFIG_DM_GPIO
+#include 
+#endif
 
 #define I2C_MAX_OFFSET_LEN 4
 
+enum {
+   PIN_SDA = 0,
+   PIN_SCL,
+   PIN_COUNT,
+};
+
 /* Useful debugging function */
 void i2c_dump_msgs(struct i2c_msg *msg, int nmsgs)
 {
@@ -445,20 +455,110 @@ int i2c_get_chip_offset_len(struct udevice *dev)
return chip->offset_len;
 }
 
+#ifdef CONFIG_DM_GPIO
+static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)
+{
+   if (bit)
+   dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
+   else
+   dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
+  GPIOD_ACTIVE_LOW |
+  GPIOD_IS_OUT_ACTIVE);
+}
+
+static int i2c_gpio_get_pin(struct gpio_desc *pin)
+{
+   return dm_gpio_get_value(pin);
+}
+
+static int i2c_deblock_gpio_loop(struct gpio_desc *sda_pin,
+struct gpio_desc *scl_pin)
+{
+   int counter = 9;
+   int ret = 0;
+
+   i2c_gpio_set_pin(sda_pin, 1);
+   i2c_gpio_set_pin(scl_pin, 1);
+   udelay(5);
+
+   /*  Toggle SCL until slave release SDA */
+   while (counter-- >= 0) {
+   i2c_gpio_set_pin(scl_pin, 1);
+   udelay(5);
+   i2c_gpio_set_pin(scl_pin, 0);
+   udelay(5);
+   if (i2c_gpio_get_pin(sda_pin))
+   break;
+   }
+
+   /* Then, send I2C stop */
+   i2c_gpio_set_pin(sda_pin, 0);
+   udelay(5);
+
+   i2c_gpio_set_pin(scl_pin, 1);
+   udelay(5);
+
+   i2c_gpio_set_pin(sda_pin, 1);
+   udelay(5);
+
+   if (!i2c_gpio_get_pin(sda_pin) || !i2c_gpio_get_pin(scl_pin))
+   ret = -EREMOTEIO;
+
+   return ret;
+}
+
+static int i2c_deblock_gpio(struct udevice *bus)
+{
+   struct gpio_desc gpios[PIN_COUNT];
+   int ret, ret0;
+
+   ret = gpio_request_list_by_name(bus, "gpios", gpios,
+   ARRAY_SIZE(gpios), GPIOD_IS_IN);
+   if (ret != ARRAY_SIZE(gpios)) {
+   debug("%s: I2C Node '%s' has no 'gpios' property %s\n",
+ __func__, dev_read_name(bus), bus->name);
+   if (ret >= 0) {
+   gpio_free_list(bus, gpios, ret);
+   ret = -ENOENT;
+   }
+   goto out;
+   }
+
+   ret = pinctrl_select_state(bus, "gpio");
+   if (ret) {
+   debug("%s: I2C Node '%s' has no 'gpio' pinctrl state. %s\n",
+ __func__, dev_read_name(bus), bus->name);
+   goto out_no_pinctrl;
+   }
+
+   ret0 = i2c_deblock_gpio_loop([PIN_SDA], [PIN_SCL]);
+
+   ret = pinctrl_select_state(bus, "default");
+   if (ret) {
+   debug("%s: I2C Node '%s' has no 'default' pinctrl state. %s\n",
+ __func__, dev_read_name(bus), bus->name);
+   }
+
+   ret = !ret ? ret0 : ret;
+
+out_no_pinctrl:
+   gpio_free_list(bus, gpios, ARRAY_SIZE(gpios));
+out:
+   return ret;
+}
+#else
+static int i2c_deblock_gpio(struct udevice *bus)
+{
+   return -ENOSYS;
+}
+#endif // CONFIG_DM_GPIO
+
 int i2c_deblock(struct udevice *bus)
 {
struct dm_i2c_ops *ops = i2c_get_ops(bus);
 
-   /*
-* We could implement a software deblocking here if we could get
-* access to the GPIOs used by I2C, and switch them to GPIO mode
-* and then back to I2C. This is somewhat beyond our powers in
-* driver model at present, so for now just fail.
-*
-* See https://patchwork.ozlabs.org/patch/399040/
-*/
if (!ops->deblock)
-   return -ENOSYS;
+   return i2c_deblock_gpio(bus);
 
return ops->deblock(bus);
 }
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 26/36] rockchip: rk1108: remove rockchip timer for sys timer

2018-03-27 Thread Alexander Kochetkov

The question is: does rk3066 and rk3188 have arch timer? If no, than removing 
rk_timer
will break u-boot for these chips.

And my comment was about global timer, not arch timer. And I failed to enable 
arch
timer for rk3188 in the kernel.

Alexander.

> 27 марта 2018 г., в 19:07, Alexander Kochetkov <al.koc...@gmail.com> 
> написал(а):
> 
>> 
>> 27 марта 2018 г., в 12:29, Kever Yang <kever.y...@rock-chips.com> написал(а):
>> 
>> We use ARM arch timer instead.
> 
> Hi, Kever!
> 
> Just let you know, that arch timer rate on rk3066 and rk3188 depends on CPU 
> frequency.
> I’ve made patch[1] for fixing that in kernel.
> If u-boot do arm clock changes after timer initialization, timer can provide 
> inaccurate delays.
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clocksource/rockchip_timer.c?id=5e0a39d0f727b35c8b7ef56ba0724c8ceb006297
> 
> Alexander.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 26/36] rockchip: rk1108: remove rockchip timer for sys timer

2018-03-27 Thread Alexander Kochetkov


> 27 марта 2018 г., в 12:29, Kever Yang  написал(а):
> 
> We use ARM arch timer instead.

Hi, Kever!

Just let you know, that arch timer rate on rk3066 and rk3188 depends on CPU 
frequency.
I’ve made patch[1] for fixing that in kernel.
If u-boot do arm clock changes after timer initialization, timer can provide 
inaccurate delays.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clocksource/rockchip_timer.c?id=5e0a39d0f727b35c8b7ef56ba0724c8ceb006297

Alexander.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 26/36] rockchip: rk1108: remove rockchip timer for sys timer

2018-03-30 Thread Alexander Kochetkov

Hello, Kevel!

I can confirm, that rk3188 doesn’t have arch timer. I made test, see below.

By the way, could you tell what git head to use to apply your patch series?
I want to test other changes as well.

I failed to apply to this one head:

commit eef11acebaa48e241e9187c717dc92d3e175c119
Author: Tom Rini <tr...@konsulko.com>
Date:   Mon Jan 29 20:12:33 2018 -0500

Prepare v2018.03-rc1

Signed-off-by: Tom Rini <tr...@konsulko.com>

I took get_ticks() code from arch_timer.c into board file and tried to
execute it:

diff --git a/arch/arm/mach-rockchip/rk3188-board.c 
b/arch/arm/mach-rockchip/rk3188-board.c
index fc58aeb..b5d0984 100644
--- a/arch/arm/mach-rockchip/rk3188-board.c
+++ b/arch/arm/mach-rockchip/rk3188-board.c
@@ -25,9 +25,28 @@ __weak int rk_board_late_init(void)
return 0;
 }
 
+#define CONFIG_SYS_HZ_CLOCK2400
+
+ulong arch_tbl = 0;
+ulong arch_tbu = 0;
+ulong arch_timer_rate_hz = CONFIG_SYS_HZ_CLOCK / CONFIG_SYS_HZ;
+
+unsigned long long arch_get_ticks(void)
+{
+   ulong nowl, nowu;
+
+   asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (nowl), "=r" (nowu));
+
+   arch_tbl = nowl;
+   arch_tbu = nowu;
+
+   return (((unsigned long long)arch_tbu) << 32) | arch_tbl;
+}
+
 int board_late_init(void)
 {
struct rk3188_grf *grf;
+   ulong val0, val1;
 
setup_boot_mode();
grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
@@ -40,6 +59,12 @@ int board_late_init(void)
NOC_REMAP_MASK << NOC_REMAP_SHIFT);
}
 
+   val0 = arch_get_ticks();
+   udelay(100);
+   val1 = arch_get_ticks();
+
+   pr_err("val0 %lu; val1 %lu\n", val0, val1);
+
return rk_board_late_init();
 }

And I get undefined instruction error on rk3188 board:

undefined instruction
pc : [<9ff760d6>]  lr : [<9ff76129>]
reloc pc : [<600010d6>]lr : [<60001129>]
sp : 9df669d8  ip : 9df66918 fp : 0017
r10: 6003d648  r9 : 9df6cee8 r8 : 10080228
r7 : 9ffb0654  r6 : 9ffb05e4 r5 : 9ffb0658  r4 : 3ff75000
r3 : 10001000  r2 : 8000 r1 : 20008000  r0 : 20008000
Flags: nzcv  IRQs off  FIQs off  Mode SVC_32

Regards,
Alexander.

> 28 марта 2018 г., в 5:33, Kever Yang <kever.y...@rock-chips.com> написал(а):
> 
> Hi Alexander,
> 
> 
> On 03/28/2018 12:21 AM, Alexander Kochetkov wrote:
>> The question is: does rk3066 and rk3188 have arch timer? If no, than 
>> removing rk_timer
>> will break u-boot for these chips.
> 
> Thanks for your comment, I will double check about if this two chips
> have arch
> timer, I think it should be, but I don't have boards now.
> 
> Thanks,
> - Kever
>> 
>> And my comment was about global timer, not arch timer. And I failed to 
>> enable arch
>> timer for rk3188 in the kernel.
>> 
>> Alexander.
>> 
>>> 27 марта 2018 г., в 19:07, Alexander Kochetkov <al.koc...@gmail.com> 
>>> написал(а):
>>> 
>>>> 27 марта 2018 г., в 12:29, Kever Yang <kever.y...@rock-chips.com> 
>>>> написал(а):
>>>> 
>>>> We use ARM arch timer instead.
>>> Hi, Kever!
>>> 
>>> Just let you know, that arch timer rate on rk3066 and rk3188 depends on CPU 
>>> frequency.
>>> I’ve made patch[1] for fixing that in kernel.
>>> If u-boot do arm clock changes after timer initialization, timer can 
>>> provide inaccurate delays.
>>> 
>>> [1] 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clocksource/rockchip_timer.c?id=5e0a39d0f727b35c8b7ef56ba0724c8ceb006297
>>> 
>>> Alexander.
>> 
> 
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] Moving the rk_board_late_init() hook to a device-model based implementation.

2018-03-20 Thread Alexander Kochetkov


> 20 марта 2018 г., в 17:03, Kever Yang  написал(а):
> 
> Maybe we can discuss this again after I send my patches.

Hello, Kever!
Please cc me on your patch series.

Regards,
Alexander.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] dm: i2c: implement gpio-based I2C deblock

2018-03-23 Thread Alexander Kochetkov


> 23 марта 2018 г., в 10:35, Heiko Schocher  написал(а):
> 
> Just triggered an travis build (not finsihed yet), and your patch drops
> errors for arm926ejs boards:
> 
> https://travis-ci.org/hsdenx/u-boot-i2c/jobs/357258790
> 
> So, can you do a full travis build for your patch before sending a v2
> and fix the errors it triggers?

Hi Heiko!

Thanks for review.

I’ll fix warnings and travis build errors. But I don’t know how to run full 
travis build.
Can you provide a link to page describing how to run travis build?

Regards,
Alexander.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-02-26 Thread Alexander Kochetkov


> 26 февр. 2018 г., в 12:43, Dr. Philipp Tomsich 
>  написал(а):
> 
> However, the GRF drivers first need to be cleaned up (i.e. the enums for the
> IOMUX definitions need to move into the pinctrl drivers), so only the 
> structure
> definitions remain in the GRF drivers.
> 
> Some of this has happened lately for some of the more recent chips, but there
> hasn’t been a larger effort to do it:
> e.g. commit 301fff4e574d373a139dd47aceabc5b4259873da for the RK3328
> 
> It would be great if you’d pick this task up for the 3036 and 3188.

I'll do that.

One more note. grf_rk3188.h also contain GRF_SOC_CON bit definitions.
They also have to renamed for consistency and to avoid conflicts.
For example, RKI2C0_SEL_SHIFT to RK3188_RKI2C0_SEL_SHIFT.

grf_rk3036.h doesn’t contain GRF_SOC_CON bit definition at all.
And I don’t have rk3036 based board. I can find TRM for rk3036 and
make definitions, but I can’t be shoure they will be valid. But 
RKI2Cx_SEL_SHIFT is the same for both chips.

> From your description this sounds like a (secondary) reset-bit for the i2c 
> controller.
> With a *-u-boot.dtsi, you could even model the GRF-offset and the bit-number 
> via
> the device tree.  The only drawback from this is that resets are (not yet) 
> automatically
> processed when probing the device in the driver model core implementation.

Actually it is impossible to reset I2C using that pin. So it is not reset 
logically.
And DT should be consistent between linux and u-boot, so adding reset node to
u-boot DT also require to add DT node to linux.

I think, It’s better to hide that thing inside driver and forget about it.

Regards,
Alexander.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] rockchip: clk: rk3188: update dpll settings to make EMAC work

2018-02-26 Thread Alexander Kochetkov

The patch set dpll settings for 300MHz to values used by binary
blob[1]. With new values dpll still generate 300MHz clock, but
EMAC work. Probably with new values dpll generate more stable clock.

dpll on rk3188 provide clocks to DDR and EMAC. With current
dpll settings EMAC doesn't work on radxa rock. EMAC sends packets
to network, but it doesn't receive anything. ifconfig shows a lot
of framing errors.

[1] https://github.com/linux-rockchip/u-boot-rockchip/blob/u-boot-rk3288/
tools/rk_tools/3188_LPDDR2_300MHz_DDR3_300MHz_20130830.bin

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/clk/rockchip/clk_rk3188.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk_rk3188.c 
b/drivers/clk/rockchip/clk_rk3188.c
index 6451c95..f674e60 100644
--- a/drivers/clk/rockchip/clk_rk3188.c
+++ b/drivers/clk/rockchip/clk_rk3188.c
@@ -123,7 +123,7 @@ static int rkclk_configure_ddr(struct rk3188_cru *cru, 
struct rk3188_grf *grf,
   unsigned int hz, bool has_bwadj)
 {
static const struct pll_div dpll_cfg[] = {
-   {.nf = 25, .nr = 2, .no = 1},
+   {.nf = 75, .nr = 1, .no = 6},
{.nf = 400, .nr = 9, .no = 2},
{.nf = 500, .nr = 9, .no = 2},
{.nf = 100, .nr = 3, .no = 1},
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/2] rockchip: pinctrl: rk3036: Move the iomux definitions into pinctrl-driver

2018-02-26 Thread Alexander Kochetkov

Clean the iomux definitions at grf_rk3036.h, and move them into
pinctrl-driver for resolving the compiling error of redefinition.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 arch/arm/include/asm/arch-rockchip/grf_rk3036.h |  409 --
 drivers/pinctrl/rockchip/pinctrl_rk3036.c   |  410 +++
 2 files changed, 410 insertions(+), 409 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3036.h 
b/arch/arm/include/asm/arch-rockchip/grf_rk3036.h
index d995b7d..eaae10b 100644
--- a/arch/arm/include/asm/arch-rockchip/grf_rk3036.h
+++ b/arch/arm/include/asm/arch-rockchip/grf_rk3036.h
@@ -80,413 +80,4 @@ struct rk3036_grf {
 };
 check_member(rk3036_grf, sdmmc_det_cnt, 0x304);
 
-/* GRF_GPIO0A_IOMUX */
-enum {
-   GPIO0A3_SHIFT   = 6,
-   GPIO0A3_MASK= 1 << GPIO0A3_SHIFT,
-   GPIO0A3_GPIO= 0,
-   GPIO0A3_I2C1_SDA,
-
-   GPIO0A2_SHIFT   = 4,
-   GPIO0A2_MASK= 1 << GPIO0A2_SHIFT,
-   GPIO0A2_GPIO= 0,
-   GPIO0A2_I2C1_SCL,
-
-   GPIO0A1_SHIFT   = 2,
-   GPIO0A1_MASK= 3 << GPIO0A1_SHIFT,
-   GPIO0A1_GPIO= 0,
-   GPIO0A1_I2C0_SDA,
-   GPIO0A1_PWM2,
-
-   GPIO0A0_SHIFT   = 0,
-   GPIO0A0_MASK= 3 << GPIO0A0_SHIFT,
-   GPIO0A0_GPIO= 0,
-   GPIO0A0_I2C0_SCL,
-   GPIO0A0_PWM1,
-};
-
-/* GRF_GPIO0B_IOMUX */
-enum {
-   GPIO0B6_SHIFT   = 12,
-   GPIO0B6_MASK= 3 << GPIO0B6_SHIFT,
-   GPIO0B6_GPIO= 0,
-   GPIO0B6_MMC1_D3,
-   GPIO0B6_I2S1_SCLK,
-
-   GPIO0B5_SHIFT   = 10,
-   GPIO0B5_MASK= 3 << GPIO0B5_SHIFT,
-   GPIO0B5_GPIO= 0,
-   GPIO0B5_MMC1_D2,
-   GPIO0B5_I2S1_SDI,
-
-   GPIO0B4_SHIFT   = 8,
-   GPIO0B4_MASK= 3 << GPIO0B4_SHIFT,
-   GPIO0B4_GPIO= 0,
-   GPIO0B4_MMC1_D1,
-   GPIO0B4_I2S1_LRCKTX,
-
-   GPIO0B3_SHIFT   = 6,
-   GPIO0B3_MASK= 3 << GPIO0B3_SHIFT,
-   GPIO0B3_GPIO= 0,
-   GPIO0B3_MMC1_D0,
-   GPIO0B3_I2S1_LRCKRX,
-
-   GPIO0B1_SHIFT   = 2,
-   GPIO0B1_MASK= 3 << GPIO0B1_SHIFT,
-   GPIO0B1_GPIO= 0,
-   GPIO0B1_MMC1_CLKOUT,
-   GPIO0B1_I2S1_MCLK,
-
-   GPIO0B0_SHIFT   = 0,
-   GPIO0B0_MASK= 3,
-   GPIO0B0_GPIO= 0,
-   GPIO0B0_MMC1_CMD,
-   GPIO0B0_I2S1_SDO,
-};
-
-/* GRF_GPIO0C_IOMUX */
-enum {
-   GPIO0C4_SHIFT   = 8,
-   GPIO0C4_MASK= 1 << GPIO0C4_SHIFT,
-   GPIO0C4_GPIO= 0,
-   GPIO0C4_DRIVE_VBUS,
-
-   GPIO0C3_SHIFT   = 6,
-   GPIO0C3_MASK= 1 << GPIO0C3_SHIFT,
-   GPIO0C3_GPIO= 0,
-   GPIO0C3_UART0_CTSN,
-
-   GPIO0C2_SHIFT   = 4,
-   GPIO0C2_MASK= 1 << GPIO0C2_SHIFT,
-   GPIO0C2_GPIO= 0,
-   GPIO0C2_UART0_RTSN,
-
-   GPIO0C1_SHIFT   = 2,
-   GPIO0C1_MASK= 1 << GPIO0C1_SHIFT,
-   GPIO0C1_GPIO= 0,
-   GPIO0C1_UART0_SIN,
-
-
-   GPIO0C0_SHIFT   = 0,
-   GPIO0C0_MASK= 1 << GPIO0C0_SHIFT,
-   GPIO0C0_GPIO= 0,
-   GPIO0C0_UART0_SOUT,
-};
-
-/* GRF_GPIO0D_IOMUX */
-enum {
-   GPIO0D4_SHIFT   = 8,
-   GPIO0D4_MASK= 1 << GPIO0D4_SHIFT,
-   GPIO0D4_GPIO= 0,
-   GPIO0D4_SPDIF,
-
-   GPIO0D3_SHIFT   = 6,
-   GPIO0D3_MASK= 1 << GPIO0D3_SHIFT,
-   GPIO0D3_GPIO= 0,
-   GPIO0D3_PWM3,
-
-   GPIO0D2_SHIFT   = 4,
-   GPIO0D2_MASK= 1 << GPIO0D2_SHIFT,
-   GPIO0D2_GPIO= 0,
-   GPIO0D2_PWM0,
-};
-
-/* GRF_GPIO1A_IOMUX */
-enum {
-   GPIO1A5_SHIFT   = 10,
-   GPIO1A5_MASK= 1 << GPIO1A5_SHIFT,
-   GPIO1A5_GPIO= 0,
-   GPIO1A5_I2S_SDI,
-
-   GPIO1A4_SHIFT   = 8,
-   GPIO1A4_MASK= 1 << GPIO1A4_SHIFT,
-   GPIO1A4_GPIO= 0,
-   GPIO1A4_I2S_SD0,
-
-   GPIO1A3_SHIFT   = 6,
-   GPIO1A3_MASK= 1 << GPIO1A3_SHIFT,
-   GPIO1A3_GPIO= 0,
-   GPIO1A3_I2S_LRCKTX,
-
-   GPIO1A2_SHIFT   = 4,
-   GPIO1A2_MASK= 3 << GPIO1A2_SHIFT,
-   GPIO1A2_GPIO= 0,
-   GPIO1A2_I2S_LRCKRX,
-   GPIO1A2_PWM1_0,
-
-   GPIO1A1_SHIFT   = 2,
-   GPIO1A1_MASK= 1 << GPIO1A1_SHIFT,
-   GPIO1A1_GPIO= 0,
-   GPIO1A1_I2S_SCLK,
-
-   GPIO1A0_SHIFT   = 0,
-   GPIO1A0_MASK= 1 << GPIO1A0_SHIFT,
-   GPIO1A0_G

[U-Boot] [PATCH 0/2] Move the iomux definitions into pinctrl-driver

2018-02-26 Thread Alexander Kochetkov

Two patches similar to commit 301fff4e574d373a139dd47aceabc5b4259873da.

Alexander Kochetkov (2):
  rockchip: pinctrl: rk3036: Move the iomux definitions into
pinctrl-driver
  rockchip: pinctrl: rk3188: Move the iomux definitions into
pinctrl-driver

 arch/arm/include/asm/arch-rockchip/grf_rk3036.h |  409 --
 arch/arm/include/asm/arch-rockchip/grf_rk3188.h |  380 -
 drivers/pinctrl/rockchip/pinctrl_rk3036.c   |  410 +++
 drivers/pinctrl/rockchip/pinctrl_rk3188.c   |  380 +
 4 files changed, 790 insertions(+), 789 deletions(-)

-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 2/2] rockchip: pinctrl: rk3188: Move the iomux definitions into pinctrl-driver

2018-02-26 Thread Alexander Kochetkov

Clean the iomux definitions at grf_rk3188.h, and move them into
pinctrl-driver for resolving the compiling error of redefinition.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 arch/arm/include/asm/arch-rockchip/grf_rk3188.h |  380 ---
 drivers/pinctrl/rockchip/pinctrl_rk3188.c   |  380 +++
 2 files changed, 380 insertions(+), 380 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3188.h 
b/arch/arm/include/asm/arch-rockchip/grf_rk3188.h
index ce7bac5..905288e 100644
--- a/arch/arm/include/asm/arch-rockchip/grf_rk3188.h
+++ b/arch/arm/include/asm/arch-rockchip/grf_rk3188.h
@@ -69,386 +69,6 @@ struct rk3188_grf {
 };
 check_member(rk3188_grf, flash_cmd_p, 0x01a4);
 
-/* GRF_GPIO0D_IOMUX */
-enum {
-   GPIO0D7_SHIFT   = 14,
-   GPIO0D7_MASK= 1,
-   GPIO0D7_GPIO= 0,
-   GPIO0D7_SPI1_CSN0,
-
-   GPIO0D6_SHIFT   = 12,
-   GPIO0D6_MASK= 1,
-   GPIO0D6_GPIO= 0,
-   GPIO0D6_SPI1_CLK,
-
-   GPIO0D5_SHIFT   = 10,
-   GPIO0D5_MASK= 1,
-   GPIO0D5_GPIO= 0,
-   GPIO0D5_SPI1_TXD,
-
-   GPIO0D4_SHIFT   = 8,
-   GPIO0D4_MASK= 1,
-   GPIO0D4_GPIO= 0,
-   GPIO0D4_SPI0_RXD,
-
-   GPIO0D3_SHIFT   = 6,
-   GPIO0D3_MASK= 3,
-   GPIO0D3_GPIO= 0,
-   GPIO0D3_FLASH_CSN3,
-   GPIO0D3_EMMC_RSTN_OUT,
-
-   GPIO0D2_SHIFT   = 4,
-   GPIO0D2_MASK= 3,
-   GPIO0D2_GPIO= 0,
-   GPIO0D2_FLASH_CSN2,
-   GPIO0D2_EMMC_CMD,
-
-   GPIO0D1_SHIFT   = 2,
-   GPIO0D1_MASK= 1,
-   GPIO0D1_GPIO= 0,
-   GPIO0D1_FLASH_CSN1,
-
-   GPIO0D0_SHIFT   = 0,
-   GPIO0D0_MASK= 3,
-   GPIO0D0_GPIO= 0,
-   GPIO0D0_FLASH_DQS,
-   GPIO0D0_EMMC_CLKOUT
-};
-
-/* GRF_GPIO1A_IOMUX */
-enum {
-   GPIO1A7_SHIFT   = 14,
-   GPIO1A7_MASK= 3,
-   GPIO1A7_GPIO= 0,
-   GPIO1A7_UART1_RTS_N,
-   GPIO1A7_SPI0_CSN0,
-
-   GPIO1A6_SHIFT   = 12,
-   GPIO1A6_MASK= 3,
-   GPIO1A6_GPIO= 0,
-   GPIO1A6_UART1_CTS_N,
-   GPIO1A6_SPI0_CLK,
-
-   GPIO1A5_SHIFT   = 10,
-   GPIO1A5_MASK= 3,
-   GPIO1A5_GPIO= 0,
-   GPIO1A5_UART1_SOUT,
-   GPIO1A5_SPI0_TXD,
-
-   GPIO1A4_SHIFT   = 8,
-   GPIO1A4_MASK= 3,
-   GPIO1A4_GPIO= 0,
-   GPIO1A4_UART1_SIN,
-   GPIO1A4_SPI0_RXD,
-
-   GPIO1A3_SHIFT   = 6,
-   GPIO1A3_MASK= 1,
-   GPIO1A3_GPIO= 0,
-   GPIO1A3_UART0_RTS_N,
-
-   GPIO1A2_SHIFT   = 4,
-   GPIO1A2_MASK= 1,
-   GPIO1A2_GPIO= 0,
-   GPIO1A2_UART0_CTS_N,
-
-   GPIO1A1_SHIFT   = 2,
-   GPIO1A1_MASK= 1,
-   GPIO1A1_GPIO= 0,
-   GPIO1A1_UART0_SOUT,
-
-   GPIO1A0_SHIFT   = 0,
-   GPIO1A0_MASK= 1,
-   GPIO1A0_GPIO= 0,
-   GPIO1A0_UART0_SIN,
-};
-
-/* GRF_GPIO1B_IOMUX */
-enum {
-   GPIO1B7_SHIFT   = 14,
-   GPIO1B7_MASK= 1,
-   GPIO1B7_GPIO= 0,
-   GPIO1B7_SPI0_CSN1,
-
-   GPIO1B6_SHIFT   = 12,
-   GPIO1B6_MASK= 3,
-   GPIO1B6_GPIO= 0,
-   GPIO1B6_SPDIF_TX,
-   GPIO1B6_SPI1_CSN1,
-
-   GPIO1B5_SHIFT   = 10,
-   GPIO1B5_MASK= 3,
-   GPIO1B5_GPIO= 0,
-   GPIO1B5_UART3_RTS_N,
-   GPIO1B5_RESERVED,
-
-   GPIO1B4_SHIFT   = 8,
-   GPIO1B4_MASK= 3,
-   GPIO1B4_GPIO= 0,
-   GPIO1B4_UART3_CTS_N,
-   GPIO1B4_GPS_RFCLK,
-
-   GPIO1B3_SHIFT   = 6,
-   GPIO1B3_MASK= 3,
-   GPIO1B3_GPIO= 0,
-   GPIO1B3_UART3_SOUT,
-   GPIO1B3_GPS_SIG,
-
-   GPIO1B2_SHIFT   = 4,
-   GPIO1B2_MASK= 3,
-   GPIO1B2_GPIO= 0,
-   GPIO1B2_UART3_SIN,
-   GPIO1B2_GPS_MAG,
-
-   GPIO1B1_SHIFT   = 2,
-   GPIO1B1_MASK= 3,
-   GPIO1B1_GPIO= 0,
-   GPIO1B1_UART2_SOUT,
-   GPIO1B1_JTAG_TDO,
-
-   GPIO1B0_SHIFT   = 0,
-   GPIO1B0_MASK= 3,
-   GPIO1B0_GPIO= 0,
-   GPIO1B0_UART2_SIN,
-   GPIO1B0_JTAG_TDI,
-};
-
-/* GRF_GPIO1D_IOMUX */
-enum {
-   GPIO1D7_SHIFT   = 14,
-   GPIO1D7_MASK= 1,
-   GPIO1D7_GPIO= 0,
-   GPIO1D7_I2C4_SCL,
-
-   GPIO1D6_SHIFT   = 12,
-   GPIO1D6_MASK= 1,
-   GPIO1D6_GPIO= 0,
-   GPIO1D6_I2C4_SDA,
-
-   GPIO1D5_SHIFT   = 10,
-   GPIO1D

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

2018-02-26 Thread Alexander Kochetkov

This one patch not needed. Just found, that the code already implemented in the 
pinctrl_rk3188_i2c_config().
It’s mystery for me why pinctrl is not called for i2c DT nodes automatically. 
If I do explicit call
"ret = pinctrl_request_noflags(pinctrl, PERIPH_ID_I2C0);» all will work.

Is this configuration feature or something I should fix in code?

Regards,
Alexander.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] rockchip: rk3188: add rk_board_late_init() hook

2018-02-26 Thread Alexander Kochetkov

All other rockchip boards have rk_board_late_init() hook,
so add it to rk3188 boards also.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 arch/arm/mach-rockchip/rk3188-board.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/rk3188-board.c 
b/arch/arm/mach-rockchip/rk3188-board.c
index 916d18f..fc58aeb 100644
--- a/arch/arm/mach-rockchip/rk3188-board.c
+++ b/arch/arm/mach-rockchip/rk3188-board.c
@@ -20,6 +20,11 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+__weak int rk_board_late_init(void)
+{
+   return 0;
+}
+
 int board_late_init(void)
 {
struct rk3188_grf *grf;
@@ -35,7 +40,7 @@ int board_late_init(void)
NOC_REMAP_MASK << NOC_REMAP_SHIFT);
}
 
-   return 0;
+   return rk_board_late_init();
 }
 
 int board_init(void)
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] dm: i2c: implement gpio-based I2C deblock

2018-03-02 Thread Alexander Kochetkov

The commit extract gpio description from device tree,
setup pins and toggle them until I2C slave device
release SDA.

Any comments? Ideas?

Could someone review the patch and tell that should
I do with it in order to bring the patch to u-boot?

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/i2c/i2c-uclass.c |   95 +-
 1 file changed, 93 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index 920811a..a451e41 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -11,6 +11,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #define I2C_MAX_OFFSET_LEN 4
 
@@ -445,9 +447,96 @@ int i2c_get_chip_offset_len(struct udevice *dev)
return chip->offset_len;
 }
 
+static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)
+{
+   if (bit)
+   dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
+   else
+   dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
+  GPIOD_ACTIVE_LOW |
+  GPIOD_IS_OUT_ACTIVE);
+}
+
+static int i2c_gpio_get_pin(struct gpio_desc *pin)
+{
+   return dm_gpio_get_value(pin);
+}
+
+static void i2c_deblock_gpio_run(struct gpio_desc *sda_pin, struct gpio_desc 
*scl_pin)
+{
+   int counter = 16;
+
+   i2c_gpio_set_pin(sda_pin, 1);
+   i2c_gpio_set_pin(scl_pin, 1);
+   udelay(5);
+
+   while (counter-- >= 0) {
+   i2c_gpio_set_pin(scl_pin, 1);
+   udelay(5);
+   i2c_gpio_set_pin(scl_pin, 0);
+   udelay(5);
+   if (i2c_gpio_get_pin(sda_pin))
+   break;
+   }
+
+   i2c_gpio_set_pin(sda_pin, 0);
+   udelay(5);
+
+   i2c_gpio_set_pin(scl_pin, 1);
+   udelay(5);
+
+   i2c_gpio_set_pin(sda_pin, 1);
+   udelay(5);
+}
+
+enum {
+   PIN_SDA = 0,
+   PIN_SCL,
+   PIN_COUNT,
+};
+
+static int i2c_deblock_gpio(struct udevice *bus)
+{
+   struct gpio_desc gpios[PIN_COUNT];
+   int ret;
+
+   ret = gpio_request_list_by_name(bus, "gpios", gpios,
+   ARRAY_SIZE(gpios), GPIOD_IS_IN);
+   if (ret != ARRAY_SIZE(gpios)) {
+   debug("%s: I2C Node '%s' has no 'gpios' property %s\n", 
__func__,
+ dev_read_name(bus), bus->name);
+   if (ret >= 0) {
+   gpio_free_list(bus, gpios, ret);
+   ret = -ENOENT;
+   }
+   goto out;
+   }
+
+   ret = pinctrl_select_state(bus, "gpio");
+   if (ret) {
+   debug("%s: I2C Node '%s' has no 'gpio' pinctrl state. %s\n", 
__func__,
+ dev_read_name(bus), bus->name);
+   goto out_no_pinctrl;
+   }
+
+   i2c_deblock_gpio_run([PIN_SDA], [PIN_SCL]);
+
+   ret = pinctrl_select_state(bus, "default");
+   if (ret) {
+   debug("%s: I2C Node '%s' has no 'default' pinctrl state. %s\n", 
__func__,
+ dev_read_name(bus), bus->name);
+   }
+
+out_no_pinctrl:
+   gpio_free_list(bus, gpios, ARRAY_SIZE(gpios));
+out:
+   return ret;
+}
+
 int i2c_deblock(struct udevice *bus)
 {
struct dm_i2c_ops *ops = i2c_get_ops(bus);
+   int ret;
 
/*
 * We could implement a software deblocking here if we could get
@@ -457,8 +546,10 @@ int i2c_deblock(struct udevice *bus)
 *
 * See https://patchwork.ozlabs.org/patch/399040/
 */
-   if (!ops->deblock)
-   return -ENOSYS;
+   if (!ops->deblock) {
+   ret = i2c_deblock_gpio(bus);
+   return ret ? -ENOSYS : 0;
+   }
 
return ops->deblock(bus);
 }
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 32/36] rockchip: remove rk_timer

2019-03-29 Thread Alexander Kochetkov

Hello, Kever!

Please keep rk_timer.c for rk3188 and other legacy chips. There is no ARM 
generic timer in
this SoC. This SoC only have private timers.

see 
https://community.arm.com/developer/ip-products/processors/f/cortex-a-forum/1449/generic-timer-on-cortex-a7-cortex-a9

> 27 марта 2018 г., в 12:29, Kever Yang  написал(а):
> 
> We do not use rk_timer.c now, remove it.
> 
> Signed-off-by: Kever Yang 
> ---

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [PATCH 2/2] rockchip: clk: rk3188: enable bwadj for rk3188 DPLL

2020-06-27 Thread Alexander Kochetkov

Hi Kever,

Strange… Then I tested a year ago I saw, that writing into bwadj registers had 
no effect
for some PLLs. But now I did another test. See patch and output. Looks like 
rk3188 allow
writing into bwadj fields.


So I do something like 'priv->has_bwadj = 1'  in the rk3188_clk_probe() and 
send try 2.

--
--- a/drivers/clk/rockchip/clk_rk3188.c
+++ b/drivers/clk/rockchip/clk_rk3188.c
@@ -91,7 +91,7 @@ static int rkclk_set_pll(struct rk3188_cru *cru, enum 
rk_clk_id clk_id,
uint vco_hz = OSC_HZ / 1000 * div->nf / div->nr * 1000;
uint output_hz = vco_hz / div->no;
 
-   debug("PLL at %x: nf=%d, nr=%d, no=%d, vco=%u Hz, output=%u Hz\n",
+   printf("PLL at %x: nf=%d, nr=%d, no=%d, vco=%u Hz, output=%u Hz\n",
  (uint)pll, div->nf, div->nr, div->no, vco_hz, output_hz);
assert(vco_hz >= VCO_MIN_HZ && vco_hz <= VCO_MAX_HZ &&
   output_hz >= OUTPUT_MIN_HZ && output_hz <= OUTPUT_MAX_HZ &&
@@ -105,8 +105,12 @@ static int rkclk_set_pll(struct rk3188_cru *cru, enum 
rk_clk_id clk_id,
 ((div->nr - 1) << CLKR_SHIFT) | (div->no - 1));
rk_clrsetreg(>con1, CLKF_MASK, div->nf - 1);
 
-   if (has_bwadj)
+   if (has_bwadj) {
+   printf("before: %p: con2=%x, clr=%x, set=%x\n",
+   >con2, readl(>con2), PLL_BWADJ_MASK, (div->nf 
>> 1) - 1);
rk_clrsetreg(>con2, PLL_BWADJ_MASK, (div->nf >> 1) - 1);
+   printf("after: %p: con2=%x\n", >con2, readl(>con2));
+   }
 
udelay(10);
 
@@ -552,7 +556,9 @@ static int rk3188_clk_probe(struct udevice *dev)
priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
if (IS_ERR(priv->grf))
return PTR_ERR(priv->grf);
-   priv->has_bwadj = (type == RK3188A_CRU) ? 1 : 0;
+   printf("type %d\n", type);
+   //priv->has_bwadj = (type == RK3188A_CRU) ? 1 : 0;
+   priv->has_bwadj = 1;
 —
type 0
PLL at 2030: nf=64, nr=1, no=2, vco=153600 Hz, output=76800 Hz
before: 2038: con2=94, clr=fff, set=1f
after: 2038: con2=1f
PLL at 2020: nf=32, nr=1, no=2, vco=76800 Hz, output=38400 Hz
before: 2028: con2=63, clr=fff, set=f
after: 2028: con2=f
PLL at 2000: nf=50, nr=1, no=2, vco=12 Hz, output=6 Hz
before: 2008: con2=18, clr=fff, set=18
after: 2008: con2=18
PLL at 2010: nf=75, nr=1, no=4, vco=18 Hz, output=45000 Hz
before: 2018: con2=10b, clr=fff, set=24
after: 2018: con2=24



> 27 июня 2020 г., в 18:17, Kever Yang  написал(а):
> 
> Hi Alex,
> 
> I think it will be better to update the rk3188_clk_probe() function 
> instead of
> 
> what you have modified if the RK3188 and RK3188A has the same PLL(I'm not sure
> 
> about it now).
> 
> 
> Thanks,
> 
> - Kever
> 
> On 2020/6/22 下午9:17, Alexander Kochetkov wrote:
>> Empirically, I found that DPLL on rk3188 has bwadj registers.
>> Configuring DPLL with bwadj increase DPLL stability. Because
>> of DPLL provide clock for ethernet, enabling bwaj reduces
>> the number of errors on the ethernet.
>> 
>> Signed-off-by: Alexander Kochetkov 
>> ---
>>  drivers/clk/rockchip/clk_rk3188.c | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/clk/rockchip/clk_rk3188.c 
>> b/drivers/clk/rockchip/clk_rk3188.c
>> index 4fc5c78563..ee5782d25d 100644
>> --- a/drivers/clk/rockchip/clk_rk3188.c
>> +++ b/drivers/clk/rockchip/clk_rk3188.c
>> @@ -117,7 +117,7 @@ static int rkclk_set_pll(struct rk3188_cru *cru, enum 
>> rk_clk_id clk_id,
>>  }
>>static int rkclk_configure_ddr(struct rk3188_cru *cru, struct rk3188_grf 
>> *grf,
>> -   unsigned int hz, bool has_bwadj)
>> +   unsigned int hz)
>>  {
>>  static const struct pll_div dpll_cfg[] = {
>>  {.nf = 75, .nr = 1, .no = 6},
>> @@ -149,7 +149,8 @@ static int rkclk_configure_ddr(struct rk3188_cru *cru, 
>> struct rk3188_grf *grf,
>>  rk_clrsetreg(>cru_mode_con, DPLL_MODE_MASK << DPLL_MODE_SHIFT,
>>   DPLL_MODE_SLOW << DPLL_MODE_SHIFT);
>>  -   rkclk_set_pll(cru, CLK_DDR, _cfg[cfg], has_bwadj);
>> +/* rk3188 and rk3188a DPLL has bwadj */
>> +rkclk_set_pll(cru, CLK_DDR, _cfg[cfg], 1);
>>  /* wait for pll lock */
>>  while (!(readl(>soc_status0) & SOCSTS_DPLL_LOCK))
>> @@ -504,8 +505,7 @@ static ulong rk3188_clk_set_rate(struct clk *clk, ulong 
>> rate)
>> priv->has_bwadj);
>>  break;
>>  case CLK_DDR:
>> -new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate,
>> -   priv->has_bwadj);
>> +new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate);
>>  break;
>>  case HCLK_EMMC:
>>  case HCLK_SDMMC:
> 
>

Re: [PATCH] rockchip: i2c: fix switch to new implementation for rk3188

2020-06-27 Thread Alexander Kochetkov

To make clear, there is kernel driver i2c-rk3x.c.
For rk3066 it write bits in the GRF word at offset 0x154. See [1] and [2].

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-rk3x.c#n1236
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-rk3x.c#n1137

In u-boot there is include file cru_rk3036.h [3]. If this include file valid 
for rk3066, than offset 0x154 correspond
to soc_con3 register. But all documentation I found for 30xx SoC clarify that 
I2C switch bits located in the
soc_con1 registers.

So I don’t know correct location for I2C switch bits.

[3] 
https://github.com/u-boot/u-boot/blob/master/arch/arm/include/asm/arch-rockchip/cru_rk3036.h


> 27 июня 2020 г., в 17:17, Kever Yang  написал(а):
> 
> +David,
> 
> Hi David,
> 
> Could you help to commend on this?
> 
> 
> Hi Alex,
> 
> Thanks for your patch.
> 
> On 2020/6/22 下午9:06, Alexander Kochetkov wrote:
>> The commit e7ae4cf27a6d 'pinctrl: rockchip: Add common rockchip
>> pinctrl driver' dropped rk3188_pinctrl_request operation, that
>> did switching to new implementation.
>> 
>> This commit implement switching to new implementation using
>> writing bits to GRF.
>> 
>> I don't have rk3060
> rk3066
>>  board to test, so switching implemented
>> as a stub returning -ENOSYS.
>> 
>> Signed-off-by: Alexander Kochetkov 
>> ---
>>  drivers/i2c/rk_i2c.c | 42 +++---
>>  1 file changed, 31 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c
>> index 32b2ee8578..ad3c66843b 100644
>> --- a/drivers/i2c/rk_i2c.c
>> +++ b/drivers/i2c/rk_i2c.c
>> @@ -15,6 +15,8 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include 
>>  #include 
>>  @@ -41,6 +43,7 @@ enum {
>>   */
>>  struct rk_i2c_soc_data {
>>  int controller_type;
>> +int (*switch_to_new_type)(int bus_nr);
>>  };
>>static inline void rk_i2c_get_div(int div, int *divh, int *divl)
>> @@ -388,11 +391,33 @@ static int rockchip_i2c_ofdata_to_platdata(struct 
>> udevice *bus)
>>  return 0;
>>  }
>>  +static int rockchip_i2c_switch_to_new_type_rk3066(int bus_nr)
>> +{
>> +return -ENOSYS;
>> +}
>> +
>> +static int rockchip_i2c_switch_to_new_type_rk3188(int bus_nr)
>> +{
>> +struct rk3188_grf *grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
>> +
>> +if (grf == NULL)
>> +return -ENOSYS;
>> +
>> +if (bus_nr < 0 || bus_nr > (RKI2C4_SEL_SHIFT - RKI2C0_SEL_SHIFT))
>> +return -EINVAL;
>> +
>> +/* enable new i2c controller */
>> +rk_clrsetreg(>soc_con1,
>> + 1 << (RKI2C0_SEL_SHIFT + bus_nr),
>> + 1 << (RKI2C0_SEL_SHIFT + bus_nr));
>> +
>> +return 0;
>> +}
>> +
>>  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;
>>  @@ -408,17 +433,10 @@ static int rockchip_i2c_probe(struct udevice *bus)
>>  return ret;
>>  }
>>  -   ret = uclass_get_device(UCLASS_PINCTRL, 0, );
>> -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) {
>> +ret = soc_data->switch_to_new_type(bus_nr);
>> +if (ret < 0) {
>>  debug("%s: Failed to switch I2C to new type %s: %d\n",
>> -__func__, bus->name, ret);
>> + __func__, bus->name, ret);
>>  return ret;
>>  }
>>  }
>> @@ -433,10 +451,12 @@ static const struct dm_i2c_ops rockchip_i2c_ops = {
>>static const struct rk_i2c_soc_data rk3066_soc_data = {
>>  .controller_type = RK_I2C_LEGACY,
>> +.switch_to_new_type = rockchip_i2c_switch_to_new_type_rk3066,
>>  };
>>static const struct rk_i2c_soc_data rk3188_soc_data = {
>>  .controller_type = RK_I2C_LEGACY,
>> +.switch_to_new_type = rockchip_i2c_switch_to_new_type_rk3188,
>>  };
>>static const struct rk_i2c_soc_data rk3228_soc_data = {
> 
>

[PATCH 0/2] v2: rr3188: change APP to 600MHz and enable bwadj for DPLL

2020-06-22 Thread Alexander Kochetkov

Hello!
Here are two patches I found usefull for rk3188.

Changes in v2:
Add u-boot@lists.denx.de as addressee.

Alexander Kochetkov (2):
  rockchip: clk: rk3188: change APLL to safe 600MHz
  rockchip: clk: rk3188: enable bwadj for rk3188 DPLL

 drivers/clk/rockchip/clk_rk3188.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH 1/2] rockchip: clk: rk3188: change APLL to safe 600MHz

2020-06-22 Thread Alexander Kochetkov

The commit 84a6a27ae3ff ("rockchip: rk3188: init CPU freq in clock
driver") changed ARM clock from 600MHz to 1600MHz. It made boot
unstable due to the fact that PMIC at the start generates insufficient
voltage for operation. See also: commit f4f57c58b589 ("rockchip:
rk3188: Setup the armclk in spl").

Fixes commit 84a6a27ae3ff ("rockchip: rk3188: init CPU freq in clock
driver").

Signed-off-by: Alexander Kochetkov 
---
 drivers/clk/rockchip/clk_rk3188.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk_rk3188.c 
b/drivers/clk/rockchip/clk_rk3188.c
index b193ac913e..4fc5c78563 100644
--- a/drivers/clk/rockchip/clk_rk3188.c
+++ b/drivers/clk/rockchip/clk_rk3188.c
@@ -564,7 +564,8 @@ static int rk3188_clk_probe(struct udevice *dev)
rkclk_init(priv->cru, priv->grf, priv->has_bwadj);
 
/* Init CPU frequency */
-   rkclk_configure_cpu(priv->cru, priv->grf, APLL_HZ, priv->has_bwadj);
+   rkclk_configure_cpu(priv->cru, priv->grf, APLL_SAFE_HZ,
+   priv->has_bwadj);
 #endif
 
return 0;
-- 
2.17.1

[PATCH 2/2] rockchip: clk: rk3188: enable bwadj for rk3188 DPLL

2020-06-22 Thread Alexander Kochetkov

Empirically, I found that DPLL on rk3188 has bwadj registers.
Configuring DPLL with bwadj increase DPLL stability. Because
of DPLL provide clock for ethernet, enabling bwaj reduces
the number of errors on the ethernet.

Signed-off-by: Alexander Kochetkov 
---
 drivers/clk/rockchip/clk_rk3188.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/rockchip/clk_rk3188.c 
b/drivers/clk/rockchip/clk_rk3188.c
index 4fc5c78563..ee5782d25d 100644
--- a/drivers/clk/rockchip/clk_rk3188.c
+++ b/drivers/clk/rockchip/clk_rk3188.c
@@ -117,7 +117,7 @@ static int rkclk_set_pll(struct rk3188_cru *cru, enum 
rk_clk_id clk_id,
 }
 
 static int rkclk_configure_ddr(struct rk3188_cru *cru, struct rk3188_grf *grf,
-  unsigned int hz, bool has_bwadj)
+  unsigned int hz)
 {
static const struct pll_div dpll_cfg[] = {
{.nf = 75, .nr = 1, .no = 6},
@@ -149,7 +149,8 @@ static int rkclk_configure_ddr(struct rk3188_cru *cru, 
struct rk3188_grf *grf,
rk_clrsetreg(>cru_mode_con, DPLL_MODE_MASK << DPLL_MODE_SHIFT,
 DPLL_MODE_SLOW << DPLL_MODE_SHIFT);
 
-   rkclk_set_pll(cru, CLK_DDR, _cfg[cfg], has_bwadj);
+   /* rk3188 and rk3188a DPLL has bwadj */
+   rkclk_set_pll(cru, CLK_DDR, _cfg[cfg], 1);
 
/* wait for pll lock */
while (!(readl(>soc_status0) & SOCSTS_DPLL_LOCK))
@@ -504,8 +505,7 @@ static ulong rk3188_clk_set_rate(struct clk *clk, ulong 
rate)
   priv->has_bwadj);
break;
case CLK_DDR:
-   new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate,
-  priv->has_bwadj);
+   new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate);
break;
case HCLK_EMMC:
case HCLK_SDMMC:
-- 
2.17.1

[PATCH] rockchip: i2c: fix switch to new implementation for rk3188

2020-06-22 Thread Alexander Kochetkov

The commit e7ae4cf27a6d 'pinctrl: rockchip: Add common rockchip
pinctrl driver' dropped rk3188_pinctrl_request operation, that
did switching to new implementation.

This commit implement switching to new implementation using
writing bits to GRF.

I don't have rk3060 board to test, so switching implemented
as a stub returning -ENOSYS.

Signed-off-by: Alexander Kochetkov 
---
 drivers/i2c/rk_i2c.c | 42 +++---
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c
index 32b2ee8578..ad3c66843b 100644
--- a/drivers/i2c/rk_i2c.c
+++ b/drivers/i2c/rk_i2c.c
@@ -15,6 +15,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 
@@ -41,6 +43,7 @@ enum {
  */
 struct rk_i2c_soc_data {
int controller_type;
+   int (*switch_to_new_type)(int bus_nr);
 };
 
 static inline void rk_i2c_get_div(int div, int *divh, int *divl)
@@ -388,11 +391,33 @@ static int rockchip_i2c_ofdata_to_platdata(struct udevice 
*bus)
return 0;
 }
 
+static int rockchip_i2c_switch_to_new_type_rk3066(int bus_nr)
+{
+   return -ENOSYS;
+}
+
+static int rockchip_i2c_switch_to_new_type_rk3188(int bus_nr)
+{
+   struct rk3188_grf *grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
+
+   if (grf == NULL)
+   return -ENOSYS;
+
+   if (bus_nr < 0 || bus_nr > (RKI2C4_SEL_SHIFT - RKI2C0_SEL_SHIFT))
+   return -EINVAL;
+
+   /* enable new i2c controller */
+   rk_clrsetreg(>soc_con1,
+1 << (RKI2C0_SEL_SHIFT + bus_nr),
+1 << (RKI2C0_SEL_SHIFT + bus_nr));
+
+   return 0;
+}
+
 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;
 
@@ -408,17 +433,10 @@ static int rockchip_i2c_probe(struct udevice *bus)
return ret;
}
 
-   ret = uclass_get_device(UCLASS_PINCTRL, 0, );
-   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) {
+   ret = soc_data->switch_to_new_type(bus_nr);
+   if (ret < 0) {
debug("%s: Failed to switch I2C to new type %s: %d\n",
-   __func__, bus->name, ret);
+__func__, bus->name, ret);
return ret;
}
}
@@ -433,10 +451,12 @@ static const struct dm_i2c_ops rockchip_i2c_ops = {
 
 static const struct rk_i2c_soc_data rk3066_soc_data = {
.controller_type = RK_I2C_LEGACY,
+   .switch_to_new_type = rockchip_i2c_switch_to_new_type_rk3066,
 };
 
 static const struct rk_i2c_soc_data rk3188_soc_data = {
.controller_type = RK_I2C_LEGACY,
+   .switch_to_new_type = rockchip_i2c_switch_to_new_type_rk3188,
 };
 
 static const struct rk_i2c_soc_data rk3228_soc_data = {
-- 
2.17.1

[PATCH] rockchip: rk3188: Fix back to BROM boot

2020-06-22 Thread Alexander Kochetkov

Move the setting for noc remap out of SPL code. Changing
noc remap inside SPL results in breaking back to BROM
boot.

Fixes commit c14fe2a8e192 ("rockchip: rk3188: Move SoC
one time setting into arch_cpu_init()").

Signed-off-by: Alexander Kochetkov 
---
 arch/arm/mach-rockchip/rk3188/rk3188.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3188/rk3188.c 
b/arch/arm/mach-rockchip/rk3188/rk3188.c
index 1b012f7f67..4115f95d77 100644
--- a/arch/arm/mach-rockchip/rk3188/rk3188.c
+++ b/arch/arm/mach-rockchip/rk3188/rk3188.c
@@ -73,15 +73,32 @@ int arch_cpu_init(void)
 BYPASSSEL_MASK | BYPASSDMEN_MASK,
 1 << BYPASSSEL_SHIFT | 1 << BYPASSDMEN_SHIFT);
 #endif
+   return 0;
+}
+#endif
+
+__weak int rk3188_board_late_init(void)
+{
+   return 0;
+}
+
+int rk_board_late_init(void)
+{
+   struct rk3188_grf *grf;
+
+   grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
+   if (IS_ERR(grf)) {
+   pr_err("grf syscon returned %ld\n", PTR_ERR(grf));
+   return 0;
+   }
 
/* enable noc remap to mimic legacy loaders */
rk_clrsetreg(>soc_con0,
 NOC_REMAP_MASK << NOC_REMAP_SHIFT,
 NOC_REMAP_MASK << NOC_REMAP_SHIFT);
 
-   return 0;
+   return rk3188_board_late_init();
 }
-#endif
 
 #ifdef CONFIG_SPL_BUILD
 static int setup_led(void)
-- 
2.17.1

Re: [PATCH] i2c: correct I2C deblock logic

2023-03-23 Thread Alexander Kochetkov

Hello Haibo Chen!

Setting GPIOD_ACTIVE_LOW has no effect. It filtered out by 
dm_gpio_set_dir_flags().
> 
> 
> if (bit)
> - dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
> + dm_gpio_set_dir_flags(pin, GPIOD_IS_IN |
> +GPIOD_ACTIVE_LOW);

Here in original code GPIOD_ACTIVE_LOW has not effect.

else
dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
GPIOD_ACTIVE_LOW |
GPIOD_IS_OUT_ACTIVE);


This code actually fix problem with your DTS-settings:
> + return !dm_gpio_get_value(pin);


Can you debug and check using oscilloscope the code like this:

static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)
{
ulong flags;
ulong active;

if (bit) {
dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
} else {
dm_gpio_get_flags(pin, );
active = (flags & GPIOD_ACTIVE_LOW) ? GPIOD_IS_OUT_ACTIVE : 0;
dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | active);
}
}

static int i2c_gpio_get_pin(struct gpio_desc *pin)
{
 ulong flags;
 int value;

 value = dm_gpio_get_value(pin);
 return (flags & GPIOD_ACTIVE_LOW) ? !value : value;
}


> 10 февр. 2023 г., в 12:27, haibo.c...@nxp.com написал(а):
> 
> From: Haibo Chen 
> 
> Current code use dm_gpio_get_value() to get SDA and SCL value, and the
> value depends on the flag GPIOD_ACTIVE_LOW. When toggle SCL to wait
> slave release SDA, the SDA are config as GPIOD_IS_IN, and whether contain
> the GPIOD_ACTIVE_LOW depends on the DTS setting. Usually, for I2C GPIO,
> we use GPIOD_ACTIVE_LOW flag in DTS, so if the SDA is in low level, then
> dm_gpio_get_value() will return 1, current code logic will stop the SCL
> toggle wrongly, cause the I2C deblock not work as expect.
> 
> This patch force set the GPIOD_ACTIVE_LOW for both GPIOD_IS_IN and
> GPIOD_IS_OUT, and make the return value of i2c_gpio_get_pin() eaqual to
> the physical voltage logic level.
> 
> Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock")
> Signed-off-by: Haibo Chen 
> ---
> drivers/i2c/i2c-uclass.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> index 8d9a89ed89..4dc707c659 100644
> --- a/drivers/i2c/i2c-uclass.c
> +++ b/drivers/i2c/i2c-uclass.c
> @@ -505,7 +505,8 @@ uint i2c_get_chip_addr_offset_mask(struct udevice *dev)
> static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)
> {
> if (bit)
> - dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
> + dm_gpio_set_dir_flags(pin, GPIOD_IS_IN |
> +   GPIOD_ACTIVE_LOW);
> else
> dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
>   GPIOD_ACTIVE_LOW |
> @@ -514,7 +515,7 @@ static void i2c_gpio_set_pin(struct gpio_desc *pin, int 
> bit)
> 
> static int i2c_gpio_get_pin(struct gpio_desc *pin)
> {
> - return dm_gpio_get_value(pin);
> + return !dm_gpio_get_value(pin);
> }
> 
> int i2c_deblock_gpio_loop(struct gpio_desc *sda_pin,
> -- 
> 2.34.1
>

Re: [PATCH] i2c: correct I2C deblock logic

2023-03-23 Thread Alexander Kochetkov

Or even simpler. Like your original patch. If we take into accout Patrik’s 
comment from another message:

> but if I assume that GPIO_ACTIVE_HIGH is NOT activated in DT

static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)
{
   if (bit) {
   dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
   } else {
   dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
   }
}

static int i2c_gpio_get_pin(struct gpio_desc *pin)
{
return !dm_gpio_get_value(pin);
}


> 23 марта 2023 г., в 11:43, Alexander Kochetkov  
> написал(а):
> 
> Hello Haibo Chen!
> 
> Setting GPIOD_ACTIVE_LOW has no effect. It filtered out by 
> dm_gpio_set_dir_flags().
>> 
>> 
>> if (bit)
>> - dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
>> + dm_gpio_set_dir_flags(pin, GPIOD_IS_IN |
>> +GPIOD_ACTIVE_LOW);
> 
> Here in original code GPIOD_ACTIVE_LOW has not effect.
> 
> else
>dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
>GPIOD_ACTIVE_LOW |
>GPIOD_IS_OUT_ACTIVE);
> 
> 
> This code actually fix problem with your DTS-settings:
>> + return !dm_gpio_get_value(pin);
> 
> 
> Can you debug and check using oscilloscope the code like this:
> 
> static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)
> {
>ulong flags;
>ulong active;
> 
>if (bit) {
>dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
>} else {
>dm_gpio_get_flags(pin, );
>active = (flags & GPIOD_ACTIVE_LOW) ? GPIOD_IS_OUT_ACTIVE : 0;
>dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | active);
>}
> }
> 
> static int i2c_gpio_get_pin(struct gpio_desc *pin)
> {
> ulong flags;
> int value;
> 
> value = dm_gpio_get_value(pin);
> return (flags & GPIOD_ACTIVE_LOW) ? !value : value;
> }
> 
> 
>> 10 февр. 2023 г., в 12:27, haibo.c...@nxp.com написал(а):
>> 
>> From: Haibo Chen 
>> 
>> Current code use dm_gpio_get_value() to get SDA and SCL value, and the
>> value depends on the flag GPIOD_ACTIVE_LOW. When toggle SCL to wait
>> slave release SDA, the SDA are config as GPIOD_IS_IN, and whether contain
>> the GPIOD_ACTIVE_LOW depends on the DTS setting. Usually, for I2C GPIO,
>> we use GPIOD_ACTIVE_LOW flag in DTS, so if the SDA is in low level, then
>> dm_gpio_get_value() will return 1, current code logic will stop the SCL
>> toggle wrongly, cause the I2C deblock not work as expect.
>> 
>> This patch force set the GPIOD_ACTIVE_LOW for both GPIOD_IS_IN and
>> GPIOD_IS_OUT, and make the return value of i2c_gpio_get_pin() eaqual to
>> the physical voltage logic level.
>> 
>> Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock")
>> Signed-off-by: Haibo Chen 
>> ---
>> drivers/i2c/i2c-uclass.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
>> index 8d9a89ed89..4dc707c659 100644
>> --- a/drivers/i2c/i2c-uclass.c
>> +++ b/drivers/i2c/i2c-uclass.c
>> @@ -505,7 +505,8 @@ uint i2c_get_chip_addr_offset_mask(struct udevice *dev)
>> static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)
>> {
>> if (bit)
>> - dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
>> + dm_gpio_set_dir_flags(pin, GPIOD_IS_IN |
>> +   GPIOD_ACTIVE_LOW);
>> else
>> dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
>>  GPIOD_ACTIVE_LOW |
>> @@ -514,7 +515,7 @@ static void i2c_gpio_set_pin(struct gpio_desc *pin, int 
>> bit)
>> 
>> static int i2c_gpio_get_pin(struct gpio_desc *pin)
>> {
>> - return dm_gpio_get_value(pin);
>> + return !dm_gpio_get_value(pin);
>> }
>> 
>> int i2c_deblock_gpio_loop(struct gpio_desc *sda_pin,
>> -- 
>> 2.34.1
>> 
>

Re: [PATCH] i2c: correct I2C deblock logic

2023-03-23 Thread Alexander Kochetkov

> If use this code, must make sure dts use GPIO_ACTIVE_LOW, otherwise 
> dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE) will config 
> gpio output high, this is not what we want.

Patrik told that if we don’t have GPIO_ACTIVE_LOW  in the DTS, this is the case 
where hardware has some sort of
signal inversion inside. May be there is no such hardware now.
When we set gpio output active, it became high inside hardware, hardware set 
i2c line low.
If you think that previous version of my today’s patch is more correct, try it.

> 
> And here use  !dm_gpio_get_value(pin), this already hit use GPIO_ACTIVE_LOW, 
> why not directly use GPIOD_ACTIVE_LOW
> when call dm_gpio_set_dir_flags, this make code more readable, and make sure 
> the code logic can work and do not depends
> on dts setting.

Patric told, that we must not use GPIOD_ACTIVE_LOW in client code. So you must 
drop it. From any patch you send.
GPIOD_ACTIVE_LOW is not applied to gpio flags when you call 
dm_gpio_set_dir_flags(). It is configured from DTS.
You think it is applied, but actually not. It’s your finding. Here your words:

> i2c_gpio_set_pin-> dm_gpio_set_dir_flags-> dm_gpio_clrset_flags, it only 
> clear GPIOD_MASK_DIR, this has no impact with GPIOD_ACTIVE_LOW. So this 
> GPIOD_ACTIVE_LOW keep in flags.


> 23 марта 2023 г., в 13:41, Bough Chen  написал(а):
> 
>> -Original Message-
>> From: Alexander Kochetkov mailto:al.koc...@gmail.com>>
>> Sent: 2023年3月23日 17:34
>> To: Bough Chen mailto:haibo.c...@nxp.com>>
>> Cc: h...@denx.de <mailto:h...@denx.de>; ma...@denx.de 
>> <mailto:ma...@denx.de>; u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>; 
>> dl-uboot-imx
>> mailto:uboot-...@nxp.com>>; xypron.g...@gmx.de 
>> <mailto:xypron.g...@gmx.de>
>> Subject: Re: [PATCH] i2c: correct I2C deblock logic
>> 
>> Or even simpler. Like your original patch. If we take into accout Patrik’s
>> comment from another message:
>> 
>>> but if I assume that GPIO_ACTIVE_HIGH is NOT activated in DT
>> 
>> static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit) {
>>   if (bit) {
>>   dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
>>   } else {
>>   dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
>>   }
>> }
>> 
>> static int i2c_gpio_get_pin(struct gpio_desc *pin) {
>>return !dm_gpio_get_value(pin);
>> }
> 
> 
> 
> And here use  !dm_gpio_get_value(pin), this already hit use GPIO_ACTIVE_LOW, 
> why not directly use GPIOD_ACTIVE_LOW
> when call dm_gpio_set_dir_flags, this make code more readable, and make sure 
> the code logic can work and do not depends
> on dts setting.
> 
> Best Regards
> Haibo Chen
>> 
>> 
>>> 23 марта 2023 г., в 11:43, Alexander Kochetkov 
>> написал(а):
>>> 
>>> Hello Haibo Chen!
>>> 
>>> Setting GPIOD_ACTIVE_LOW has no effect. It filtered out by
>> dm_gpio_set_dir_flags().
>>>> 
>>>> 
>>>> if (bit)
>>>> - dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
>>>> + dm_gpio_set_dir_flags(pin, GPIOD_IS_IN |
>>>> +GPIOD_ACTIVE_LOW);
>>> 
>>> Here in original code GPIOD_ACTIVE_LOW has not effect.
>>> 
>>> else
>>>   dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
>>> 
>> GPIOD_ACTIVE_LOW |
>>> 
>> GPIOD_IS_OUT_ACTIVE);
>>> 
>>> 
>>> This code actually fix problem with your DTS-settings:
>>>> + return !dm_gpio_get_value(pin);
>>> 
>>> 
>>> Can you debug and check using oscilloscope the code like this:
>>> 
>>> static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit) {
>>>   ulong flags;
>>>   ulong active;
>>> 
>>>   if (bit) {
>>>   dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
>>>   } else {
>>>   dm_gpio_get_flags(pin, );
>>>   active = (flags & GPIOD_ACTIVE_LOW) ? GPIOD_IS_OUT_ACTIVE :
>> 0;
>>>   dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | active);
>>>   }
>>> }
>>> 
>>> static int i2c_gpio_get_pin(struct gpio_desc *pin) {
>>>ulong flags;
>>>int value;
>>> 
>>>value = dm_gpio_get_value(pin);
>>>return (flags & GPIOD_ACTIVE_LOW) ? !value : value; }
>>> 
>>> 
>>>> 10 февр. 2023 г., в 12:27, haibo.c...@nxp.com написал(а):
>>>> 
>>>> From: Haibo Chen 
>>>> 
>>>> Current code use dm_gpio_get_value() to get SDA and SCL value, and
>>>>

Re: [PATCH] i2c: correct I2C deblock logic

2023-03-21 Thread Alexander Kochetkov

Hi Haibo.

Nice catch! Agree with you patch. But may be fix problem in general?
I suggest to add GPIOD_ACTIVE_LOW to the GPIOD_MASK_DIR mask to fix problems 1, 
2, 4.
Not sure if that logically correct. What do you think?

I grepped the code for ‘dm_gpio_set_dir_flags’ and found another places 
affected by the bug.

1. u-boot/drivers/spi/mxc_spi.c:
ret = dm_gpio_set_dir_flags(>cs_gpios[i],
  GPIOD_IS_OUT | GPIOD_ACTIVE_LOW);

Here GPIOD_ACTIVE_LOW is passed to dm_gpio_set_dir_flags() and filtered out by 
dm_gpio_set_dir_flags().
GPIOD_ACTIVE_LOW can be added to GPIOD_MASK_DIR mask to fix that.

2. u-boot/drivers/spi/mvebu_a3700_spi.c:
ret = dm_gpio_set_dir_flags(>cs_gpios[i],
 GPIOD_IS_OUT | GPIOD_ACTIVE_LOW);
Same as 1.

3. u-boot/drivers/phy/allwinner/phy-sun4i-usb.c:
ret = dm_gpio_set_dir_flags(>gpio_id_det,
 GPIOD_IS_IN | GPIOD_PULL_UP);

Here GPIOD_PULL_UP is passed to dm_gpio_set_dir_flags() and filtered out by 
dm_gpio_set_dir_flags().
Looks, like we have to introduce dm_gpio_set_pull_flags() and use it together 
with dm_gpio_set_dir_flags().

4. u-boot/drivers/i2c/i2c-uclass.c
dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
GPIOD_ACTIVE_LOW |
GPIOD_IS_OUT_ACTIVE);
Same as 1. Your patch cover that bug.




> 21 марта 2023 г., в 11:37, Bough Chen  написал(а):
> 
>> -Original Message-----
>> From: Alexander Kochetkov 
>> Sent: 2023年3月20日 16:03
>> To: h...@denx.de
>> Cc: Bough Chen ; ma...@denx.de;
>> u-boot@lists.denx.de; dl-uboot-imx ;
>> xypron.g...@gmx.de; Simon Glass 
>> Subject: Re: [PATCH] i2c: correct I2C deblock logic
>> 
>> Hello!
>> 
>> The patch doesn’t add new functionality to the code.
>> May be it makes code more readable. But in later case the patch description
>> should be corrected and Fixes tag removed.
> 
> Hi Alexander,
> 
> This patch not only make the code more readable, but also really fix a bug.
> In our current code, we will call i2c_gpio_set_pin(scl_pin, 1) and then 
> i2c_gpio_set_pin(scl_pin, 0).
> After the i2c_gpio_set_pin(scl_pin, 0), the flags contain GPIOD_ACTIVE_LOW.
> Though for SDA, it will first call i2c_gpio_set_pin(sda_pin, 0) then 
> i2c_gpio_set_pin(sda_pin, 1), but 
> i2c_gpio_set_pin-> dm_gpio_set_dir_flags-> dm_gpio_clrset_flags, it only 
> clear GPIOD_MASK_DIR, this has no impact with GPIOD_ACTIVE_LOW. So this 
> GPIOD_ACTIVE_LOW keep in flags.
> Then call i2c_gpio_get_pin by using dm_gpio_get_value(pin) will get the wrong 
> data, let the function i2c_deblock_gpio_loop always return -EREMOTEIO
> 
> Best Regards
> Haibo Chen
> 
>> 
>> The flag GPIOD_ACTIVE_LOW affects return value dm_gpio_get_value().
>> And return value doesn’t depends on the DTS settings. It depends on the flag
>> passed to dm_gpio_set_dir_flags(). Usually the code parses DTS and provide 
>> the
>> correct flag to the dm_gpio_set_dir_flags().
>> 
>> So the patch adds flag GPIOD_ACTIVE_LOW to the dm_gpio_set_dir_flags() and
>> as expected this negates the return value of dm_gpio_get_value(). So in 
>> order to
>> keep the code correct the patch also fixes adds negotiation to
>> dm_gpio_get_value().
>> 
>> Alexander.
>> 
>> 
>> 
>>> 20 марта 2023 г., в 10:44, Heiko Schocher  написал(а):
>>> 
>>> Hi!
>>> 
>>> On 13.03.23 03:55, Bough Chen wrote:
>>>>> -Original Message-
>>>>> From: Bough Chen 
>>>>> Sent: 2023年2月10日 17:27
>>>>> To: h...@denx.de; al.koc...@gmail.com; ma...@denx.de
>>>>> Cc: u-boot@lists.denx.de; dl-uboot-imx ;
>>>>> xypron.g...@gmx.de; Bough Chen 
>>>>> Subject: [PATCH] i2c: correct I2C deblock logic
>>>>> 
>>>>> From: Haibo Chen 
>>>>> 
>>>>> Current code use dm_gpio_get_value() to get SDA and SCL value, and
>>>>> the value depends on the flag GPIOD_ACTIVE_LOW. When toggle SCL to
>>>>> wait slave release SDA, the SDA are config as GPIOD_IS_IN, and
>>>>> whether contain the GPIOD_ACTIVE_LOW depends on the DTS setting.
>>>>> Usually, for I2C GPIO, we use GPIOD_ACTIVE_LOW flag in DTS, so if
>>>>> the SDA is in low level, then
>>>>> dm_gpio_get_value() will return 1, current code logic will stop the
>>>>> SCL toggle wrongly, cause the I2C deblock not work as expect.
>>>>> 
>>>>> This patch force set the GPIOD_ACTIVE_LOW for both GPIOD_I

Re: [PATCH] gpio: add GPIOD_ACTIVE_LOW into GPIOD_MASK_DIR

2023-03-22 Thread Alexander Kochetkov

Reviewed-by: Alexander Kochetkov 

> 22 марта 2023 г., в 14:26, haibo.c...@nxp.com написал(а):
> 
> From: Haibo Chen 
> 
> dm_gpio_set_dir_flags() will clear GPIOD_MASK_DIR and set new flags.
> But there are cases like i2c_deblock_gpio_loop() will do like this:
> 
> -first conifg GPIO(SDA) output with GPIOD_ACTIVE_LOW
> dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
>   GPIOD_ACTIVE_LOW |
>   GPIOD_IS_OUT_ACTIVE);
> 
> -then config GPIO input
> dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
> 
> -then get the GPIO input value:
> dm_gpio_get_value(pin);
> 
> When config the GPIO input, only set GPIOD_IS_IN, but unfortunately
> since the previous GPIOD_ACTIVE_LOW is not cleared, still keep in
> flags, make the value from dm_gpio_get_value() not logic correct.
> 
> So add GPIOD_ACTIVE_LOW into GPIOD_MASK_DIR to avoid this issue.
> 
> Signed-off-by: Haibo Chen 
> ---
> include/asm-generic/gpio.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index dd0bdf2315..903b237aac 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -131,7 +131,7 @@ struct gpio_desc {
> 
> /* Flags for updating the above */
> #define GPIOD_MASK_DIR (GPIOD_IS_OUT | GPIOD_IS_IN | \
> - GPIOD_IS_OUT_ACTIVE)
> + GPIOD_IS_OUT_ACTIVE | GPIOD_ACTIVE_LOW)
> #define GPIOD_MASK_DSTYPE (GPIOD_OPEN_DRAIN | GPIOD_OPEN_SOURCE)
> #define GPIOD_MASK_PULL (GPIOD_PULL_UP | GPIOD_PULL_DOWN)
> 
> -- 
> 2.34.1
>

Re: [PATCH] i2c: correct I2C deblock logic

2023-03-20 Thread Alexander Kochetkov

Hello!

The patch doesn’t add new functionality to the code.
May be it makes code more readable. But in later case the patch description 
should be corrected and
Fixes tag removed.

The flag GPIOD_ACTIVE_LOW affects return value dm_gpio_get_value().
And return value doesn’t depends on the DTS settings. It depends on
the flag passed to dm_gpio_set_dir_flags(). Usually the code parses DTS and
provide the correct flag to the dm_gpio_set_dir_flags().

So the patch adds flag GPIOD_ACTIVE_LOW to the dm_gpio_set_dir_flags() and
as expected this negates the return value of dm_gpio_get_value(). So in
order to keep the code correct the patch also fixes adds negotiation to
dm_gpio_get_value().

Alexander.



> 20 марта 2023 г., в 10:44, Heiko Schocher  написал(а):
> 
> Hi!
> 
> On 13.03.23 03:55, Bough Chen wrote:
>>> -Original Message-
>>> From: Bough Chen 
>>> Sent: 2023年2月10日 17:27
>>> To: h...@denx.de; al.koc...@gmail.com; ma...@denx.de
>>> Cc: u-boot@lists.denx.de; dl-uboot-imx ;
>>> xypron.g...@gmx.de; Bough Chen 
>>> Subject: [PATCH] i2c: correct I2C deblock logic
>>> 
>>> From: Haibo Chen 
>>> 
>>> Current code use dm_gpio_get_value() to get SDA and SCL value, and the value
>>> depends on the flag GPIOD_ACTIVE_LOW. When toggle SCL to wait slave
>>> release SDA, the SDA are config as GPIOD_IS_IN, and whether contain the
>>> GPIOD_ACTIVE_LOW depends on the DTS setting. Usually, for I2C GPIO, we use
>>> GPIOD_ACTIVE_LOW flag in DTS, so if the SDA is in low level, then
>>> dm_gpio_get_value() will return 1, current code logic will stop the SCL 
>>> toggle
>>> wrongly, cause the I2C deblock not work as expect.
>>> 
>>> This patch force set the GPIOD_ACTIVE_LOW for both GPIOD_IS_IN and
>>> GPIOD_IS_OUT, and make the return value of i2c_gpio_get_pin() eaqual to the
>>> physical voltage logic level.
>>> 
>> 
>> Hi, 
>> 
>> Any comments for this patch, just a reminder in case you miss it.
> 
> Indeed, I missed this patch, sorry!
> Your explanation sounds reasonable to me, but I wonder if nobody tapped
> into this problem... it would be good to have some test reports
> from others! Also added Simon, as he did a lot of work in this space.
> 
> Thanks!
> 
> bye,
> Heiko
>> 
>> Best Regards
>> Haibo Chen
>> 
>>> Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock")
>>> Signed-off-by: Haibo Chen 
>>> ---
>>> drivers/i2c/i2c-uclass.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c index
>>> 8d9a89ed89..4dc707c659 100644
>>> --- a/drivers/i2c/i2c-uclass.c
>>> +++ b/drivers/i2c/i2c-uclass.c
>>> @@ -505,7 +505,8 @@ uint i2c_get_chip_addr_offset_mask(struct udevice
>>> *dev)  static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)  {
>>> if (bit)
>>> -   dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
>>> +   dm_gpio_set_dir_flags(pin, GPIOD_IS_IN |
>>> +  GPIOD_ACTIVE_LOW);
>>> else
>>> dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
>>>GPIOD_ACTIVE_LOW |
>>> @@ -514,7 +515,7 @@ static void i2c_gpio_set_pin(struct gpio_desc *pin, int
>>> bit)
>>> 
>>> static int i2c_gpio_get_pin(struct gpio_desc *pin)  {
>>> -   return dm_gpio_get_value(pin);
>>> +   return !dm_gpio_get_value(pin);
>>> }
>>> 
>>> int i2c_deblock_gpio_loop(struct gpio_desc *sda_pin,
>>> --
>>> 2.34.1
>> 
> 
> -- 
> DENX Software Engineering GmbH,  Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de

Re: [PATCH v3] i2c: correct I2C deblock logic

2023-03-27 Thread Alexander Kochetkov

Hello Haibo Chen.

The patch looks fine!

Reviewed-by: Alexander Kochetkov mailto:al.koc...@gmail.com>>

> 27 марта 2023 г., в 14:21, haibo.c...@nxp.com написал(а):
> 
> From: Haibo Chen 
> 
> Current code use dm_gpio_get_value() to get SDA and SCL value, and the
> value depends on whether DTS file config the GPIO_ACTIVE_LOW. In ususal
> case for i2c GPIO, DTS need to set GPIO_ACTIVE_LOW for SCL/SDA pins. So
> here the logic is not correct.
> 
> And we must not use GPIOD_ACTIVE_LOW in client code include the
> dm_gpio_set_dir_flags(), it is DTS's responsibility for this flag. So
> remove GPIOD_ACTIVE_LOW here.
> 
> Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock")
> Signed-off-by: Haibo Chen 
> ---
> drivers/i2c/i2c-uclass.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> index 8d9a89ed89..8867a560bd 100644
> --- a/drivers/i2c/i2c-uclass.c
> +++ b/drivers/i2c/i2c-uclass.c
> @@ -508,13 +508,13 @@ static void i2c_gpio_set_pin(struct gpio_desc *pin, int 
> bit)
>   dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
>   else
>   dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
> -GPIOD_ACTIVE_LOW |
>  GPIOD_IS_OUT_ACTIVE);
> }
> 
> static int i2c_gpio_get_pin(struct gpio_desc *pin)
> {
> - return dm_gpio_get_value(pin);
> + /* DTS need config GPIO_ACTIVE_LOW */
> + return !dm_gpio_get_value(pin);
> }
> 
> int i2c_deblock_gpio_loop(struct gpio_desc *sda_pin,
> -- 
> 2.34.1
>

41 matches

Mail list logo