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

2020-06-28 Thread Heiko Schocher 

Hi Alex,

Am 29.06.2020 um 05:23 schrieb David Wu:

Hi Alexander,

Thank you for your patch, the grf header file is missing for rk3066, the GRF_SOC_CON1 offset of 3066 
is 0x154, the corresponding bit of i2c0~i2c4 is also bit11 ~ bit15.


There is currently no support for rk3066 at mainline, rk3066 is not handled here, I think it’s okay, 
so,


Reviewed-by: David Wu 


Reviewed-by: Heiko Schocher 

@Kever: Do you pickup this patch?
  (I cannot find it in patchwork, is it assigned to you?)

bye,
Heiko


在 2020/6/27 下午11:03, 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 = {













--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
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] rockchip: i2c: fix switch to new implementation for rk3188

2020-06-28 Thread David Wu 

Hi Alexander,

Thank you for your patch, the grf header file is missing for rk3066, the 
GRF_SOC_CON1 offset of 3066 is 0x154, the corresponding bit of i2c0~i2c4 
is also bit11 ~ bit15.


There is currently no support for rk3066 at mainline, rk3066 is not 
handled here, I think it’s okay, so,


Reviewed-by: David Wu 

在 2020/6/27 下午11:03, 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 = {

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 = {
> 
>

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

2020-06-27 Thread 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 = {