Re: [U-Boot] [PATCH 1/4] sunxi: video: Fix VIDEO_LCD_PANEL_I2C being enabled by default

On Sat, 2015-03-07 at 15:04 +0100, Hans de Goede wrote:
Fix a type in board/sunxi/Kconfig which caused VIDEO_LCD_PANEL_I2C to be
"typo"
+#define CONFIG_VIDEO_LCD_I2C_BUS 1 /* NA, but necessary to compile */
Hrm, should the usage sites not be either ifdef'd or excluded at the Makefile level when VIDEO_LCD_PANEL_I2C is disabled?
The only use is in if (IS_ENABLED(CONFIG_VIDEO_LCD_TL059WV5C0)) { unsigned int orig_i2c_bus = i2c_get_bus_num(); i2c_set_bus_num(CONFIG_VIDEO_LCD_I2C_BUS); i2c_reg_write(0x5c, 0x04, 0x42); /* Turn on the LCD */ i2c_set_bus_num(orig_i2c_bus); }
Is the issue that the IS_ENABLED statically false but the compiler still wants the contents to be valid?
How about a helper set_video_i2c_bus or some such which can be a nop if CONFIG_VIDEO_LCD_I2C_BUS is not defined, which would keep the ifdef out of this code?
Or at least #define it to some obviously bogus value (e.g. ~0UL).
Ian.

Hi,
On 08-03-15 10:04, Ian Campbell wrote:
On Sat, 2015-03-07 at 15:04 +0100, Hans de Goede wrote:
Fix a type in board/sunxi/Kconfig which caused VIDEO_LCD_PANEL_I2C to be
"typo"
Heh, so I made a typo in the word typo, fixed :)
+#define CONFIG_VIDEO_LCD_I2C_BUS 1 /* NA, but necessary to compile */
Hrm, should the usage sites not be either ifdef'd or excluded at the Makefile level when VIDEO_LCD_PANEL_I2C is disabled?
The only use is in if (IS_ENABLED(CONFIG_VIDEO_LCD_TL059WV5C0)) { unsigned int orig_i2c_bus = i2c_get_bus_num(); i2c_set_bus_num(CONFIG_VIDEO_LCD_I2C_BUS); i2c_reg_write(0x5c, 0x04, 0x42); /* Turn on the LCD */ i2c_set_bus_num(orig_i2c_bus); }
Is the issue that the IS_ENABLED statically false but the compiler still wants the contents to be valid?
Right.
How about a helper set_video_i2c_bus or some such which can be a nop if CONFIG_VIDEO_LCD_I2C_BUS is not defined, which would keep the ifdef out of this code?
Not a fan of that, the whole purpose of using IS_ENABLED is to have easier to read code, I do not believe that adding a wrapper helps there.
Or at least #define it to some obviously bogus value (e.g. ~0UL).
That is a good idea, I've changed it to -1 in my personal tree.
Regards,
Hans

On Tue, 2015-03-10 at 12:38 +0100, Hans de Goede wrote:
Or at least #define it to some obviously bogus value (e.g. ~0UL).
That is a good idea, I've changed it to -1 in my personal tree.
With that:
Acked-by: Ian Campbell ijc@hellion.org.uk
participants (2)
-
Hans de Goede
-
Ian Campbell