All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode
@ 2019-04-12 19:19 Trent Piepho
  2019-04-30  4:24 ` Heiko Schocher
  0 siblings, 1 reply; 8+ messages in thread
From: Trent Piepho @ 2019-04-12 19:19 UTC (permalink / raw)
  To: u-boot

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 <sriram.dash@nxp.com>
Cc: Priyanka Jain <priyanka.jain@nxp.com>
Cc: Heiko Schocher <hs@denx.de>
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
---
 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode
  2019-04-12 19:19 [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode Trent Piepho
@ 2019-04-30  4:24 ` Heiko Schocher
  2019-04-30  7:20   ` Heiko Schocher
  0 siblings, 1 reply; 8+ messages in thread
From: Heiko Schocher @ 2019-04-30  4:24 UTC (permalink / raw)
  To: u-boot

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 <sriram.dash@nxp.com>
> Cc: Priyanka Jain <priyanka.jain@nxp.com>
> Cc: Heiko Schocher <hs@denx.de>
> Signed-off-by: Trent Piepho <tpiepho@impinj.com>
> ---
>   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 <hs@denx.de>

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: hs at denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode
  2019-04-30  4:24 ` Heiko Schocher
@ 2019-04-30  7:20   ` Heiko Schocher
  2019-04-30 17:21     ` Trent Piepho
  0 siblings, 1 reply; 8+ messages in thread
From: Heiko Schocher @ 2019-04-30  7:20 UTC (permalink / raw)
  To: u-boot

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 <sriram.dash@nxp.com>
>> Cc: Priyanka Jain <priyanka.jain@nxp.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> Signed-off-by: Trent Piepho <tpiepho@impinj.com>
>> ---
>>   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, &mx6dl_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, &mx6q_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 <hs@denx.de>
> 
> 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: hs at denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode
  2019-04-30  7:20   ` Heiko Schocher
@ 2019-04-30 17:21     ` Trent Piepho
  2019-05-02  5:39       ` Heiko Schocher
  0 siblings, 1 reply; 8+ messages in thread
From: Trent Piepho @ 2019-04-30 17:21 UTC (permalink / raw)
  To: u-boot

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, &mx6dl_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, &mx6q_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, &mx6dl_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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode
  2019-04-30 17:21     ` Trent Piepho
@ 2019-05-02  5:39       ` Heiko Schocher
  2019-05-02  6:11         ` Anatolij Gustschin
  0 siblings, 1 reply; 8+ messages in thread
From: Heiko Schocher @ 2019-05-02  5:39 UTC (permalink / raw)
  To: u-boot

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, &mx6dl_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, &mx6q_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, &mx6dl_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, &bus);
  266         if (rc)
  267                 return rc;
  268         rc = dm_i2c_probe(bus, dev->addr, 0, &udev);
  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: hs at denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode
  2019-05-02  5:39       ` Heiko Schocher
@ 2019-05-02  6:11         ` Anatolij Gustschin
  2019-05-03 20:16           ` Trent Piepho
  0 siblings, 1 reply; 8+ messages in thread
From: Anatolij Gustschin @ 2019-05-02  6:11 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

On Thu, 2 May 2019 07:39:06 +0200
Heiko Schocher hs at 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode
  2019-05-02  6:11         ` Anatolij Gustschin
@ 2019-05-03 20:16           ` Trent Piepho
  2019-05-06  8:58             ` Heiko Schocher
  0 siblings, 1 reply; 8+ messages in thread
From: Trent Piepho @ 2019-05-03 20:16 UTC (permalink / raw)
  To: u-boot

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 hs at 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, &mx6dl_i2c2_pad_info);
+       setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6dl_i2c2_pad_info);
        if (is_mx6dq() || is_mx6dqp()) {
-               setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6q_i2c2_pad_info);
-               setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, &mx6q_i2c3_pad_info);
+               setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6q_i2c2_pad_info);
+               setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, &mx6q_i2c3_pad_info);
        } else {
-               setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6dl_i2c2_pad_info);
-               setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, &mx6dl_i2c3_pad_info);
+               setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6dl_i2c2_pad_info);
+               setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, &mx6dl_i2c3_pad_info);
        }
  
        setup_display();

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode
  2019-05-03 20:16           ` Trent Piepho
@ 2019-05-06  8:58             ` Heiko Schocher
  0 siblings, 0 replies; 8+ messages in thread
From: Heiko Schocher @ 2019-05-06  8:58 UTC (permalink / raw)
  To: u-boot

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 hs at 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, &mx6dl_i2c2_pad_info);
> +       setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6dl_i2c2_pad_info);
>          if (is_mx6dq() || is_mx6dqp()) {
> -               setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6q_i2c2_pad_info);
> -               setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, &mx6q_i2c3_pad_info);
> +               setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6q_i2c2_pad_info);
> +               setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, &mx6q_i2c3_pad_info);
>          } else {
> -               setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6dl_i2c2_pad_info);
> -               setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, &mx6dl_i2c3_pad_info);
> +               setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6dl_i2c2_pad_info);
> +               setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, &mx6dl_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: hs at denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-05-06  8:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-12 19:19 [U-Boot] [PATCH] i2c: mxc: Hide kconfig based control in DM_I2C mode Trent Piepho
2019-04-30  4:24 ` Heiko Schocher
2019-04-30  7:20   ` Heiko Schocher
2019-04-30 17:21     ` Trent Piepho
2019-05-02  5:39       ` Heiko Schocher
2019-05-02  6:11         ` Anatolij Gustschin
2019-05-03 20:16           ` Trent Piepho
2019-05-06  8:58             ` Heiko Schocher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.