Re: [U-Boot] [PATCH] video: Remove dependecy of I2C_EDID

2017-04-16 Thread Simon Glass 
Hi,

On 9 April 2017 at 15:30, Jernej Škrabec  wrote:
> Hi,
>
> Dne nedelja, 09. april 2017 ob 21:28:47 CEST je Simon Glass napisal(a):
>> Hi,
>>
>> On 28 March 2017 at 16:39, Jernej Skrabec  wrote:
>> > I2C_EDID currently selects DM_I2C. However, it is not needed. I2C_EDID
>> > is used for building edid.c, which doesn't even use I2C bus, and by I2C
>> > command, which knows how to use DM and old style I2C interface, so it is
>> > not directly affected by this removal.
>> >
>> > Furthermore, this selection can generate warning if DM display driver
>> > is used on platform which doesn't implement DM I2C driver (for example,
>> > sunxi platform with upcoming DM video & display driver).
>> >
>> > Patch was tested with rockchip and sunxi boards and successfully
>> > compiled exynos and tegra targets. They are the only consumers of
>> > CONFIG_DISPLAY option, which is the only one which selects I2C_EDID.
>> >
>> > Signed-off-by: Jernej Skrabec 
>> > ---
>> >
>> >  cmd/i2c.c | 10 ++
>> >  drivers/video/Kconfig |  1 -
>> >  2 files changed, 6 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/cmd/i2c.c b/cmd/i2c.c
>> > index 473153fbd4..7b6306e525 100644
>> > --- a/cmd/i2c.c
>> > +++ b/cmd/i2c.c
>> > @@ -1630,7 +1630,8 @@ static int do_sdram (cmd_tbl_t * cmdtp, int flag,
>> > int argc, char * const argv[])>
>> >   * Syntax:
>> >   * i2c edid {i2c_chip}
>> >   */
>> >
>> > -#if defined(CONFIG_I2C_EDID)
>> > +#if defined(CONFIG_I2C_EDID) && \
>> > +   (defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C))
>>
>> The correct solution here I think is to convert sunxi to DM_I2C. We
>> should not be adding new features to the old code.
>
> With the "old code" you referring to i2c command? Actually, I'm not sure if
> "i2c edid" command can be useful on most platforms. I know that rk3288 has
> multiplexed I2C controller pins with HDMI DDC pins, where this make sense. But
> for example on sunxi, in order to be useful, it would mean that dw_hdmi driver
> has to register DDC as I2C driver.
>
> I'm also not sure why "i2c edid" code knows how to use old and DM I2C
> interface when it is surrounded by a symbol, which always selects DM_I2C.
> Well, in sunxi case, that actually prevents build failure, but still produces
> unwanted warning.
>
> Otherwise I agree that converting sunxi to DM_I2C should be done and patch for
> that already exists, but it was not merged yet:
> https://patchwork.ozlabs.org/patch/734375/

Sounds good. Let's get that merged and then we don't need to worry
about the legacy I2C.

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

Re: [U-Boot] [PATCH] video: Remove dependecy of I2C_EDID

2017-04-09 Thread Jernej Škrabec 
Hi,

Dne nedelja, 09. april 2017 ob 21:28:47 CEST je Simon Glass napisal(a):
> Hi,
> 
> On 28 March 2017 at 16:39, Jernej Skrabec  wrote:
> > I2C_EDID currently selects DM_I2C. However, it is not needed. I2C_EDID
> > is used for building edid.c, which doesn't even use I2C bus, and by I2C
> > command, which knows how to use DM and old style I2C interface, so it is
> > not directly affected by this removal.
> > 
> > Furthermore, this selection can generate warning if DM display driver
> > is used on platform which doesn't implement DM I2C driver (for example,
> > sunxi platform with upcoming DM video & display driver).
> > 
> > Patch was tested with rockchip and sunxi boards and successfully
> > compiled exynos and tegra targets. They are the only consumers of
> > CONFIG_DISPLAY option, which is the only one which selects I2C_EDID.
> > 
> > Signed-off-by: Jernej Skrabec 
> > ---
> > 
> >  cmd/i2c.c | 10 ++
> >  drivers/video/Kconfig |  1 -
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/cmd/i2c.c b/cmd/i2c.c
> > index 473153fbd4..7b6306e525 100644
> > --- a/cmd/i2c.c
> > +++ b/cmd/i2c.c
> > @@ -1630,7 +1630,8 @@ static int do_sdram (cmd_tbl_t * cmdtp, int flag,
> > int argc, char * const argv[])> 
> >   * Syntax:
> >   * i2c edid {i2c_chip}
> >   */
> > 
> > -#if defined(CONFIG_I2C_EDID)
> > +#if defined(CONFIG_I2C_EDID) && \
> > +   (defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C))
> 
> The correct solution here I think is to convert sunxi to DM_I2C. We
> should not be adding new features to the old code.

With the "old code" you referring to i2c command? Actually, I'm not sure if 
"i2c edid" command can be useful on most platforms. I know that rk3288 has 
multiplexed I2C controller pins with HDMI DDC pins, where this make sense. But 
for example on sunxi, in order to be useful, it would mean that dw_hdmi driver 
has to register DDC as I2C driver.

I'm also not sure why "i2c edid" code knows how to use old and DM I2C 
interface when it is surrounded by a symbol, which always selects DM_I2C. 
Well, in sunxi case, that actually prevents build failure, but still produces 
unwanted warning.

Otherwise I agree that converting sunxi to DM_I2C should be done and patch for 
that already exists, but it was not merged yet:
https://patchwork.ozlabs.org/patch/734375/

Regards,
Jernej
> 
> >  int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> >  {
> >  
> > uint chip;
> > 
> > @@ -1665,7 +1666,7 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc,
> > char *const argv[])> 
> > return 0;
> >  
> >  }
> > 
> > -#endif /* CONFIG_I2C_EDID */
> > +#endif
> > 
> >  #ifdef CONFIG_DM_I2C
> >  static void show_bus(struct udevice *bus)
> > 
> > @@ -1936,9 +1937,10 @@ static cmd_tbl_t cmd_i2c_sub[] = {
> > 
> > defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_DM_I2C)
> > U_BOOT_CMD_MKENT(dev, 1, 1, do_i2c_bus_num, "", ""),
> >  
> >  #endif  /* CONFIG_I2C_MULTI_BUS */
> > 
> > -#if defined(CONFIG_I2C_EDID)
> > +#if defined(CONFIG_I2C_EDID) && \
> > +   (defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C))
> > 
> > U_BOOT_CMD_MKENT(edid, 1, 1, do_edid, "", ""),
> > 
> > -#endif  /* CONFIG_I2C_EDID */
> > +#endif
> > 
> > U_BOOT_CMD_MKENT(loop, 3, 1, do_i2c_loop, "", ""),
> > U_BOOT_CMD_MKENT(md, 3, 1, do_i2c_md, "", ""),
> > U_BOOT_CMD_MKENT(mm, 2, 1, do_i2c_mm, "", ""),
> > 
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 2069576958..2f340235ee 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -355,7 +355,6 @@ config VIDEO_MVEBU
> > 
> >  config I2C_EDID
> >  
> > bool "Enable EDID library"
> > 
> > -   depends on DM_I2C
> > 
> > default n
> > help
> > 
> >This enables library for accessing EDID data from an LCD panel.
> > 
> > --
> > 2.12.1
> 
> Regards,
> Simon


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

Re: [U-Boot] [PATCH] video: Remove dependecy of I2C_EDID

2017-04-09 Thread Simon Glass 
Hi,

On 28 March 2017 at 16:39, Jernej Skrabec  wrote:
> I2C_EDID currently selects DM_I2C. However, it is not needed. I2C_EDID
> is used for building edid.c, which doesn't even use I2C bus, and by I2C
> command, which knows how to use DM and old style I2C interface, so it is
> not directly affected by this removal.
>
> Furthermore, this selection can generate warning if DM display driver
> is used on platform which doesn't implement DM I2C driver (for example,
> sunxi platform with upcoming DM video & display driver).
>
> Patch was tested with rockchip and sunxi boards and successfully
> compiled exynos and tegra targets. They are the only consumers of
> CONFIG_DISPLAY option, which is the only one which selects I2C_EDID.
>
> Signed-off-by: Jernej Skrabec 
> ---
>
>  cmd/i2c.c | 10 ++
>  drivers/video/Kconfig |  1 -
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/cmd/i2c.c b/cmd/i2c.c
> index 473153fbd4..7b6306e525 100644
> --- a/cmd/i2c.c
> +++ b/cmd/i2c.c
> @@ -1630,7 +1630,8 @@ static int do_sdram (cmd_tbl_t * cmdtp, int flag, int 
> argc, char * const argv[])
>   * Syntax:
>   * i2c edid {i2c_chip}
>   */
> -#if defined(CONFIG_I2C_EDID)
> +#if defined(CONFIG_I2C_EDID) && \
> +   (defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C))

The correct solution here I think is to convert sunxi to DM_I2C. We
should not be adding new features to the old code.

>  int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>  {
> uint chip;
> @@ -1665,7 +1666,7 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char 
> *const argv[])
> return 0;
>
>  }
> -#endif /* CONFIG_I2C_EDID */
> +#endif
>
>  #ifdef CONFIG_DM_I2C
>  static void show_bus(struct udevice *bus)
> @@ -1936,9 +1937,10 @@ static cmd_tbl_t cmd_i2c_sub[] = {
> defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_DM_I2C)
> U_BOOT_CMD_MKENT(dev, 1, 1, do_i2c_bus_num, "", ""),
>  #endif  /* CONFIG_I2C_MULTI_BUS */
> -#if defined(CONFIG_I2C_EDID)
> +#if defined(CONFIG_I2C_EDID) && \
> +   (defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C))
> U_BOOT_CMD_MKENT(edid, 1, 1, do_edid, "", ""),
> -#endif  /* CONFIG_I2C_EDID */
> +#endif
> U_BOOT_CMD_MKENT(loop, 3, 1, do_i2c_loop, "", ""),
> U_BOOT_CMD_MKENT(md, 3, 1, do_i2c_md, "", ""),
> U_BOOT_CMD_MKENT(mm, 2, 1, do_i2c_mm, "", ""),
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 2069576958..2f340235ee 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -355,7 +355,6 @@ config VIDEO_MVEBU
>
>  config I2C_EDID
> bool "Enable EDID library"
> -   depends on DM_I2C
> default n
> help
>This enables library for accessing EDID data from an LCD panel.
> --
> 2.12.1
>

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

[U-Boot] [PATCH] video: Remove dependecy of I2C_EDID

2017-03-28 Thread Jernej Skrabec 
I2C_EDID currently selects DM_I2C. However, it is not needed. I2C_EDID
is used for building edid.c, which doesn't even use I2C bus, and by I2C
command, which knows how to use DM and old style I2C interface, so it is
not directly affected by this removal.

Furthermore, this selection can generate warning if DM display driver
is used on platform which doesn't implement DM I2C driver (for example,
sunxi platform with upcoming DM video & display driver).

Patch was tested with rockchip and sunxi boards and successfully
compiled exynos and tegra targets. They are the only consumers of
CONFIG_DISPLAY option, which is the only one which selects I2C_EDID.

Signed-off-by: Jernej Skrabec 
---

 cmd/i2c.c | 10 ++
 drivers/video/Kconfig |  1 -
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/cmd/i2c.c b/cmd/i2c.c
index 473153fbd4..7b6306e525 100644
--- a/cmd/i2c.c
+++ b/cmd/i2c.c
@@ -1630,7 +1630,8 @@ static int do_sdram (cmd_tbl_t * cmdtp, int flag, int 
argc, char * const argv[])
  * Syntax:
  * i2c edid {i2c_chip}
  */
-#if defined(CONFIG_I2C_EDID)
+#if defined(CONFIG_I2C_EDID) && \
+   (defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C))
 int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 {
uint chip;
@@ -1665,7 +1666,7 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char 
*const argv[])
return 0;
 
 }
-#endif /* CONFIG_I2C_EDID */
+#endif
 
 #ifdef CONFIG_DM_I2C
 static void show_bus(struct udevice *bus)
@@ -1936,9 +1937,10 @@ static cmd_tbl_t cmd_i2c_sub[] = {
defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_DM_I2C)
U_BOOT_CMD_MKENT(dev, 1, 1, do_i2c_bus_num, "", ""),
 #endif  /* CONFIG_I2C_MULTI_BUS */
-#if defined(CONFIG_I2C_EDID)
+#if defined(CONFIG_I2C_EDID) && \
+   (defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C))
U_BOOT_CMD_MKENT(edid, 1, 1, do_edid, "", ""),
-#endif  /* CONFIG_I2C_EDID */
+#endif
U_BOOT_CMD_MKENT(loop, 3, 1, do_i2c_loop, "", ""),
U_BOOT_CMD_MKENT(md, 3, 1, do_i2c_md, "", ""),
U_BOOT_CMD_MKENT(mm, 2, 1, do_i2c_mm, "", ""),
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2069576958..2f340235ee 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -355,7 +355,6 @@ config VIDEO_MVEBU
 
 config I2C_EDID
bool "Enable EDID library"
-   depends on DM_I2C
default n
help
   This enables library for accessing EDID data from an LCD panel.
-- 
2.12.1

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