Re: [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode

2019-05-06 Thread Heiko Schocher 

Hello Trent,

Am 03.05.2019 um 22:16 schrieb Trent Piepho:

On Thu, 2019-05-02 at 08:11 +0200, Anatolij Gustschin wrote:

Hi Heiko,

On Thu, 2 May 2019 07:39:06 +0200
Heiko Schocher h...@denx.de wrote:
...


@Anatolij: Is this code needed anymore, as board is moved to DM ?


...

dm_i2c_probe should initialize the i2c bus completly, also we need
not longer the:

#ifdef CONFIG_DM_I2C

and remove the else part ... or ? Can you test this change?


The DM conversion is not complete yet and I still need the
possibility to revert to non-DM code for various video tests
to complete the DM_VIDEO/DM_I2C conversion, so that video
related stuff works on i.MX boards similar as before the
conversion. Unfortunately, I do not have time for this now,
this must wait. Sorry.

Anatolij


Would something like this be ok?  It will allow removing the MXC
hardcoded speeds when using DM_I2C.  It also makes it more clear in
wandboard.c that the speed is not used when using DM_I2C.

--- a/board/wandboard/wandboard.c
+++ b/board/wandboard/wandboard.c
@@ -46,6 +46,15 @@ DECLARE_GLOBAL_DATA_PTR;
  #define ETH_PHY_AR8035_POWER   IMX_GPIO_NR(7, 13)
  #define REV_DETECTION  IMX_GPIO_NR(2, 28)
   
+/* Speed defined in Kconfig is only applicable when not using DM_I2C.  */

+#ifdef CONFIG_DM_I2C
+#define I2C1_SPEED_NON_DM  0
+#define I2C2_SPEED_NON_DM  0
+#else
+#define I2C1_SPEED_NON_DM  CONFIG_SYS_MXC_I2C1_SPEED
+#define I2C2_SPEED_NON_DM  CONFIG_SYS_MXC_I2C2_SPEED
+#endif
+
  static bool with_pmic;
   
  int dram_init(void)

@@ -463,13 +472,13 @@ int board_init(void)
 gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
   
  #if defined(CONFIG_VIDEO_IPUV3)

-   setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, _i2c2_pad_info);
+   setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, _i2c2_pad_info);
 if (is_mx6dq() || is_mx6dqp()) {
-   setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, 
_i2c2_pad_info);
-   setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, 
_i2c3_pad_info);
+   setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, _i2c2_pad_info);
+   setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, _i2c3_pad_info);
 } else {
-   setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, 
_i2c2_pad_info);
-   setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, 
_i2c3_pad_info);
+   setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, _i2c2_pad_info);
+   setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, _i2c3_pad_info);
 }
   
 setup_display();




For this special case and for me, this is ok.

If nobody objects here, I would pick this change in conjunction with
your original patch ... or you sent a formal v2 ?

Thanks!

bye,
Heiko
--
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode

2019-05-03 Thread Trent Piepho 
On Thu, 2019-05-02 at 08:11 +0200, Anatolij Gustschin wrote:
> Hi Heiko,
> 
> On Thu, 2 May 2019 07:39:06 +0200
> Heiko Schocher h...@denx.de wrote:
> ...
> > 
> > @Anatolij: Is this code needed anymore, as board is moved to DM ?
> 
> ...
> > dm_i2c_probe should initialize the i2c bus completly, also we need
> > not longer the:
> > 
> > #ifdef CONFIG_DM_I2C
> > 
> > and remove the else part ... or ? Can you test this change?
> 
> The DM conversion is not complete yet and I still need the
> possibility to revert to non-DM code for various video tests
> to complete the DM_VIDEO/DM_I2C conversion, so that video
> related stuff works on i.MX boards similar as before the
> conversion. Unfortunately, I do not have time for this now,
> this must wait. Sorry.
> 
> Anatolij

Would something like this be ok?  It will allow removing the MXC
hardcoded speeds when using DM_I2C.  It also makes it more clear in
wandboard.c that the speed is not used when using DM_I2C.

--- a/board/wandboard/wandboard.c
+++ b/board/wandboard/wandboard.c
@@ -46,6 +46,15 @@ DECLARE_GLOBAL_DATA_PTR;
 #define ETH_PHY_AR8035_POWER   IMX_GPIO_NR(7, 13)
 #define REV_DETECTION  IMX_GPIO_NR(2, 28)
  
+/* Speed defined in Kconfig is only applicable when not using DM_I2C.  */
+#ifdef CONFIG_DM_I2C
+#define I2C1_SPEED_NON_DM  0
+#define I2C2_SPEED_NON_DM  0
+#else
+#define I2C1_SPEED_NON_DM  CONFIG_SYS_MXC_I2C1_SPEED
+#define I2C2_SPEED_NON_DM  CONFIG_SYS_MXC_I2C2_SPEED
+#endif
+
 static bool with_pmic;
  
 int dram_init(void)
@@ -463,13 +472,13 @@ int board_init(void)
gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
  
 #if defined(CONFIG_VIDEO_IPUV3)
-   setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, _i2c2_pad_info);
+   setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, _i2c2_pad_info);
if (is_mx6dq() || is_mx6dqp()) {
-   setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, 
_i2c2_pad_info);
-   setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, 
_i2c3_pad_info);
+   setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, _i2c2_pad_info);
+   setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, _i2c3_pad_info);
} else {
-   setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, 
_i2c2_pad_info);
-   setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, 
_i2c3_pad_info);
+   setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, _i2c2_pad_info);
+   setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, _i2c3_pad_info);
}
  
setup_display();
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode

2019-05-02 Thread Anatolij Gustschin 
Hi Heiko,

On Thu, 2 May 2019 07:39:06 +0200
Heiko Schocher h...@denx.de wrote:
...
> 
> @Anatolij: Is this code needed anymore, as board is moved to DM ?
...
> dm_i2c_probe should initialize the i2c bus completly, also we need
> not longer the:
> 
> #ifdef CONFIG_DM_I2C
> 
> and remove the else part ... or ? Can you test this change?

The DM conversion is not complete yet and I still need the
possibility to revert to non-DM code for various video tests
to complete the DM_VIDEO/DM_I2C conversion, so that video
related stuff works on i.MX boards similar as before the
conversion. Unfortunately, I do not have time for this now,
this must wait. Sorry.

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

Re: [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode

2019-05-01 Thread Heiko Schocher 

Hello Trent,

Am 30.04.2019 um 19:21 schrieb Trent Piepho:

On Tue, 2019-04-30 at 09:20 +0200, Heiko Schocher wrote:

Am 12.04.2019 um 21:19 schrieb Trent Piepho:

These options only apply when not using DM_I2C.  When using device
trees, the dt will enable and control the speeds of the I2C
controller(s) and these configuration options have no effect.

So disable them in DM_I2C mode.  Otherwise they show up as decoys, and
make it look like one is enabling I2C controllers and setting the speed
when really it's doing nothing.



 arm:  +   wandboard
+board/wandboard/wandboard.c: In function 'board_init':
+board/wandboard/wandboard.c:466:15: error: 'CONFIG_SYS_MXC_I2C1_SPEED' 
undeclared (first use in
this function); did you mean 'CONFIG_SYS_MMC_ENV_DEV'?
+  setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, _i2c2_pad_info);
+   ^
+   CONFIG_SYS_MMC_ENV_DEV
+board/wandboard/wandboard.c:466:15: note: each undeclared identifier is 
reported only once for each
function it appears in
+board/wandboard/wandboard.c:469:16: error: 'CONFIG_SYS_MXC_I2C2_SPEED' 
undeclared (first use in
this function); did you mean 'CONFIG_SYS_MXC_I2C1_SPEED'?
+   setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, _i2c3_pad_info);
+^
+CONFIG_SYS_MXC_I2C1_SPEED
+make[2]: *** [board/wandboard/wandboard.o] Error 1
+make[1]: *** [board/wandboard] Error 2
+make: *** [sub-make] Error 2

Please check.


This is caused by a conflict with "imx6: wandboard: convert to DM_I2C",
which was applied after I sent my patch.


Ah!


I'm not sure if that commit is correct. Copying Anatolij.


added also Stefano as imx6 maintainer.


It's the only place where non-DM_I2C code uses the kconfig based
configuration of I2C.  If we look at the code in question:

setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, _i2c2_pad_info);

Seems a bit odd bus 1 is being configured with i2c*2* pad info?  But
let's take a look at setup_i2c().

int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
   struct i2c_pads_info *p)
{
 
 /* Enable i2c clock */
 ret = enable_i2c_clk(1, i2c_index);
 if (ret)
 goto err_clk;

 /* Make sure bus is idle */
 ret = force_idle_bus(p);
 if (ret)
 goto err_idle;

#ifndef CONFIG_DM_I2C
 bus_i2c_init(i2c_index, speed, slave_addr, force_idle_bus, p);
#endif
 return 0;
  
err_idle:

err_clk:
 gpio_free(p->scl.gp);
err_req:
 gpio_free(p->sda.gp);

 return ret;
}

Nothing in the elided section uses "speed".  It's only used in the
bus_i2c_init() call, and that call is not made when using DM_I2C!

So while the wandboard code references CONFIG_SYS_MXC_I2C1_SPEED when
using DM_i2C, it's never used by anything.

So I believe I'm still correct, even after the wandboard change that
CONFIG_SYS_MXC_I2C1_SPEED et al are decoys that aren't used.


Yes, I think you are correct.

@Anatolij: Is this code needed anymore, as board is moved to DM ?

If I look into board/wandboard/wandboard.c

 259 static int detect_i2c(struct display_info_t const *dev)
 260 {
 261 #ifdef CONFIG_DM_I2C
 262 struct udevice *bus, *udev;
 263 int rc;
 264
 265 rc = uclass_get_device_by_seq(UCLASS_I2C, dev->bus, );
 266 if (rc)
 267 return rc;
 268 rc = dm_i2c_probe(bus, dev->addr, 0, );
 269 if (rc)
 270 return 0;
 271 return 1;
 272 #else
 273 return (0 == i2c_set_bus_num(dev->bus)) &&
 274 (0 == i2c_probe(dev->addr));
 275 #endif
 276 }

dm_i2c_probe should initialize the i2c bus completly, also we need
not longer the:

#ifdef CONFIG_DM_I2C

and remove the else part ... or ? Can you test this change?

bye,
Heiko
--
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode

2019-04-30 Thread Trent Piepho 
On Tue, 2019-04-30 at 09:20 +0200, Heiko Schocher wrote:
> > Am 12.04.2019 um 21:19 schrieb Trent Piepho:
> > > These options only apply when not using DM_I2C.  When using device
> > > trees, the dt will enable and control the speeds of the I2C
> > > controller(s) and these configuration options have no effect.
> > > 
> > > So disable them in DM_I2C mode.  Otherwise they show up as decoys, and
> > > make it look like one is enabling I2C controllers and setting the speed
> > > when really it's doing nothing.

> arm:  +   wandboard
> +board/wandboard/wandboard.c: In function 'board_init':
> +board/wandboard/wandboard.c:466:15: error: 'CONFIG_SYS_MXC_I2C1_SPEED' 
> undeclared (first use in 
> this function); did you mean 'CONFIG_SYS_MMC_ENV_DEV'?
> +  setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, _i2c2_pad_info);
> +   ^
> +   CONFIG_SYS_MMC_ENV_DEV
> +board/wandboard/wandboard.c:466:15: note: each undeclared identifier is 
> reported only once for each 
> function it appears in
> +board/wandboard/wandboard.c:469:16: error: 'CONFIG_SYS_MXC_I2C2_SPEED' 
> undeclared (first use in 
> this function); did you mean 'CONFIG_SYS_MXC_I2C1_SPEED'?
> +   setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, _i2c3_pad_info);
> +^
> +CONFIG_SYS_MXC_I2C1_SPEED
> +make[2]: *** [board/wandboard/wandboard.o] Error 1
> +make[1]: *** [board/wandboard] Error 2
> +make: *** [sub-make] Error 2
> 
> Please check.

This is caused by a conflict with "imx6: wandboard: convert to DM_I2C",
which was applied after I sent my patch.  

I'm not sure if that commit is correct. Copying Anatolij.

It's the only place where non-DM_I2C code uses the kconfig based
configuration of I2C.  If we look at the code in question:

setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, _i2c2_pad_info);

Seems a bit odd bus 1 is being configured with i2c*2* pad info?  But
let's take a look at setup_i2c().

int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
  struct i2c_pads_info *p)
{

/* Enable i2c clock */
ret = enable_i2c_clk(1, i2c_index);
if (ret)
goto err_clk;

/* Make sure bus is idle */
ret = force_idle_bus(p);
if (ret)
goto err_idle;

#ifndef CONFIG_DM_I2C
bus_i2c_init(i2c_index, speed, slave_addr, force_idle_bus, p);
#endif
return 0;
 
err_idle:
err_clk:
gpio_free(p->scl.gp);
err_req:
gpio_free(p->sda.gp);

return ret;
}

Nothing in the elided section uses "speed".  It's only used in the
bus_i2c_init() call, and that call is not made when using DM_I2C!

So while the wandboard code references CONFIG_SYS_MXC_I2C1_SPEED when
using DM_i2C, it's never used by anything.

So I believe I'm still correct, even after the wandboard change that
CONFIG_SYS_MXC_I2C1_SPEED et al are decoys that aren't used.

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

Re: [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode

2019-04-30 Thread Heiko Schocher 

Hello Trent,

Am 30.04.2019 um 06:24 schrieb Heiko Schocher:

Hello Trent,

Am 12.04.2019 um 21:19 schrieb Trent Piepho:

These options only apply when not using DM_I2C.  When using device
trees, the dt will enable and control the speeds of the I2C
controller(s) and these configuration options have no effect.

So disable them in DM_I2C mode.  Otherwise they show up as decoys, and
make it look like one is enabling I2C controllers and setting the speed
when really it's doing nothing.

Cc: Sriram Dash 
Cc: Priyanka Jain 
Cc: Heiko Schocher 
Signed-off-by: Trent Piepho 
---
  drivers/i2c/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Your patch has checkpatch warning:


ERROR: DOS line endings
#142: FILE: drivers/i2c/Kconfig:164:
+if SYS_I2C_MXC && !DM_I2C^M$

Fixed this locally, please check furhter patches.


Also, your patch breaks travis build, see:

https://travis-ci.org/hsdenx/u-boot-i2c/jobs/526286390

   arm:  +   wandboard
+board/wandboard/wandboard.c: In function 'board_init':
+board/wandboard/wandboard.c:466:15: error: 'CONFIG_SYS_MXC_I2C1_SPEED' undeclared (first use in 
this function); did you mean 'CONFIG_SYS_MMC_ENV_DEV'?

+  setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, _i2c2_pad_info);
+   ^
+   CONFIG_SYS_MMC_ENV_DEV
+board/wandboard/wandboard.c:466:15: note: each undeclared identifier is reported only once for each 
function it appears in
+board/wandboard/wandboard.c:469:16: error: 'CONFIG_SYS_MXC_I2C2_SPEED' undeclared (first use in 
this function); did you mean 'CONFIG_SYS_MXC_I2C1_SPEED'?

+   setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, _i2c3_pad_info);
+^
+CONFIG_SYS_MXC_I2C1_SPEED
+make[2]: *** [board/wandboard/wandboard.o] Error 1
+make[1]: *** [board/wandboard] Error 2
+make: *** [sub-make] Error 2

Please check.

bye,
Heiko



diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 1ef22e6bcd..df7fc7db0a 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -161,7 +161,7 @@ config SYS_I2C_MXC
    channels and operating on standard mode upto 100 kbits/s and fast
    mode upto 400 kbits/s.
-if SYS_I2C_MXC
+if SYS_I2C_MXC && !DM_I2C
  config SYS_I2C_MXC_I2C1
  bool "NXP MXC I2C1"
  help


Reviewed-by: Heiko Schocher 

bye,
Heiko


--
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode

2019-04-29 Thread Heiko Schocher 

Hello Trent,

Am 12.04.2019 um 21:19 schrieb Trent Piepho:

These options only apply when not using DM_I2C.  When using device
trees, the dt will enable and control the speeds of the I2C
controller(s) and these configuration options have no effect.

So disable them in DM_I2C mode.  Otherwise they show up as decoys, and
make it look like one is enabling I2C controllers and setting the speed
when really it's doing nothing.

Cc: Sriram Dash 
Cc: Priyanka Jain 
Cc: Heiko Schocher 
Signed-off-by: Trent Piepho 
---
  drivers/i2c/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Your patch has checkpatch warning:


ERROR: DOS line endings
#142: FILE: drivers/i2c/Kconfig:164:
+if SYS_I2C_MXC && !DM_I2C^M$

Fixed this locally, please check furhter patches.


diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 1ef22e6bcd..df7fc7db0a 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -161,7 +161,7 @@ config SYS_I2C_MXC
  channels and operating on standard mode upto 100 kbits/s and fast
  mode upto 400 kbits/s.
  
-if SYS_I2C_MXC

+if SYS_I2C_MXC && !DM_I2C
  config SYS_I2C_MXC_I2C1
bool "NXP MXC I2C1"
help


Reviewed-by: Heiko Schocher 

bye,
Heiko
--
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode

2019-04-12 Thread Trent Piepho 
These options only apply when not using DM_I2C.  When using device
trees, the dt will enable and control the speeds of the I2C
controller(s) and these configuration options have no effect.

So disable them in DM_I2C mode.  Otherwise they show up as decoys, and
make it look like one is enabling I2C controllers and setting the speed
when really it's doing nothing.

Cc: Sriram Dash 
Cc: Priyanka Jain 
Cc: Heiko Schocher 
Signed-off-by: Trent Piepho 
---
 drivers/i2c/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 1ef22e6bcd..df7fc7db0a 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -161,7 +161,7 @@ config SYS_I2C_MXC
  channels and operating on standard mode upto 100 kbits/s and fast
  mode upto 400 kbits/s.
 
-if SYS_I2C_MXC
+if SYS_I2C_MXC && !DM_I2C
 config SYS_I2C_MXC_I2C1
bool "NXP MXC I2C1"
help
-- 
2.14.5

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