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

[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 <jernej.skrabec at siol.net> 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 <jernej.skrabec at siol.net>
> > ---
> > 
> >  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