Dan,
On 6/21/20 10:24 PM, Jacek Anaszewski wrote:
Dan,
On 6/21/20 4:12 PM, Dan Murphy wrote:
Jacek
On 6/19/20 5:10 PM, Jacek Anaszewski wrote:
Dan,
On 6/19/20 6:35 PM, Dan Murphy wrote:
Jacek
On 6/18/20 6:26 PM, Jacek Anaszewski wrote:
On 6/19/20 12:09 AM, Jacek Anaszewski wrote:
Dan,
On 6/18/20 11:44 PM, Dan Murphy wrote:
Jacek
On 6/18/20 4:21 PM, Jacek Anaszewski wrote:
Dan,
On 6/18/20 12:33 AM, Dan Murphy wrote:
Jacek
On 6/17/20 4:41 PM, Jacek Anaszewski wrote:
Dan,
On 6/17/20 9:22 PM, Dan Murphy wrote:
Pavel/Jacek
On 6/17/20 11:28 AM, kernel test robot wrote:
Hi Dan,
I love your patch! Yet something to improve:
[auto build test ERROR on pavel-linux-leds/for-next]
[cannot apply to j.anaszewski-leds/for-next]
[if your patch is applied to the wrong git tree, please drop
us a note to help
improve the system. BTW, we also suggest to use '--base'
option to specify the
base tree in git format-patch, please see
https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Dan-Murphy/Multicolor-Framework-v27/20200616-042217
base:
git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git
for-next
config: ia64-randconfig-r015-20200617 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0
make.cross ARCH=ia64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>
All errors (new ones prefixed by >>, old ones prefixed by <<):
ia64-linux-ld: drivers/leds/leds-lp55xx-common.o: in
function `lp55xx_set_mc_brightness':
drivers/leds/leds-lp55xx-common.c:146: undefined reference
to `led_mc_calc_color_components'
ia64-linux-ld: drivers/leds/leds-lp55xx-common.o: in
function `devm_led_classdev_multicolor_register':
include/linux/led-class-multicolor.h:74: undefined
reference to `devm_led_classdev_multicolor_register_ext'
vim +146 drivers/leds/leds-lp55xx-common.c
138
139 static int lp55xx_set_mc_brightness(struct
led_classdev *cdev,
140 enum led_brightness brightness)
141 {
142 struct led_classdev_mc *mc_dev =
lcdev_to_mccdev(cdev);
143 struct lp55xx_led *led =
mcled_cdev_to_led(mc_dev);
144 struct lp55xx_device_config *cfg =
led->chip->cfg;
145
> 146 led_mc_calc_color_components(&led->mc_cdev,
brightness);
147 return cfg->multicolor_brightness_fn(led);
148
Well this was a mess to figure out.
The only fix I can figure out here is to remove the
depends on LEDS_CLASS_MULTI_COLOR ||
!LEDS_CLASS_MULTI_COLOR
from each child device and add
select LEDS_CLASS_MULTI_COLOR
to the LP55XX_COMMON
This way the Multi color framework will inherit the symbol
that was set by the COMMON flag which is inherited by
majority from the child flags.
Did you try this?
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -398,6 +398,7 @@ config LEDS_LP50XX
config LEDS_LP55XX_COMMON
tristate "Common Driver for TI/National
LP5521/5523/55231/5562/8501"
depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562
|| LEDS_LP8501
+ depends on LEDS_CLASS_MULTI_COLOR ||
!LEDS_CLASS_MULTI_COLOR
depends on OF
select FW_LOADER
select FW_LOADER_USER_HELPER
Yes I did
That gave unmet dependencies.
WARNING: unmet direct dependencies detected for LEDS_LP55XX_COMMON
Depends on [m]: NEW_LEDS [=y] && (LEDS_LP5521 [=n] ||
LEDS_LP5523 [=m] || LEDS_LP5562 [=y] || LEDS_LP8501 [=y]) &&
(LEDS_CLASS_MULTI_COLOR [=m] || !LEDS_CLASS_MULTI_COLOR [=m])
&& OF [=y]
Selected by [y]:
- LEDS_LP5562 [=y] && NEW_LEDS [=y] && LEDS_CLASS [=y] &&
I2C [=y]
- LEDS_LP8501 [=y] && NEW_LEDS [=y] && LEDS_CLASS [=y] &&
I2C [=y]
Selected by [m]:
- LEDS_LP5523 [=m] && NEW_LEDS [=y] && LEDS_CLASS [=y] &&
I2C [=y] && (LEDS_CLASS_MULTI_COLOR [=m] ||
!LEDS_CLASS_MULTI_COLOR [=m])
When I was testing that yesterday I also had the same warning at
some
point of testing different Kconfig setups, but with what I
showed above
it ceased to appear. Now every time I am doing "make oldconfig" the
CONFIG_LEDS_LP55XX_COMMON=y entry gets changed to =m with the
config
from the test bot.
That is not what I saw in my testing especially after doing a
distclean
Could you please give your exact steps after "make distclean" and
copying test bot config to the kernel root directory?
Also, please share the toolchain you're using for tests.
Actually at this stage the toolchain is of lesser relevance.
I've tried once more and indeed the problem shows up.
It is caused by the driver entries doing
"select LEDS_LP55XX_COMMON".
Select sets config to "y" so it conflicts with
"depends on LEDS_CLASS_MULTI_COLOR || !LEDS_CLASS_MULTI_COLOR"
in the "config LEDS_LP55XX_COMMON".
Your proposed fix will block the possibility of building
LED_CLASS_MULTI_COLOR as a module when LP55XX drivers
are enabled so this is also not an option.
Solving this riddle will require some more thinking.
I haven't analyzed it in detail but maybe "imply" statement from
kconfig-language.rst could help somehow here.
The multicolor framework will build as a module if the LED_CLASS is
defined as a module.
See attached test_defconfig
But it will be impossible to enable CONFIG_LEDS_LP50XX without
CONFIG_LEDS_CLASS_MULTI_COLOR if you will remove
depends on LEDS_CLASS_MULTI_COLOR || !LEDS_CLASS_MULTI_COLOR.
I was not removing the dependency for the LP50xx only the LP55xx.
It was my typo, I meant LP55XX.
This is actually why the above entry was needed.
LP55XX drivers have to work also without multicolor class.
Well I am not sure how else to resolve this problem. Because the
LP55xx has multi level dependencies.
Only the LP55xx_common has the dependency on the MC framework. The
device drivers do not.
The issue is the mixing and matching of the MC fw as a module vs the
LP55XX_COMMON as a built-in.
The simplest solution is to avoid selecting LP55XX_COMMON in favor
of making child drivers depending on it. Of course, maybe there exists
some trickier option to keep the selection, but I would have to play
a bit more with it to be sure.
Yep, this seems to be the only viable solution.
My proposed change to the Kconfig:
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index adfa762cb291..10de7d237ffd 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -397,10 +397,10 @@ config LEDS_LP50XX
config LEDS_LP55XX_COMMON
tristate "Common Driver for TI/National
LP5521/5523/55231/5562/8501"
- depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501
depends on OF
select FW_LOADER
select FW_LOADER_USER_HELPER
+ depends on LEDS_CLASS_MULTI_COLOR || !LEDS_CLASS_MULTI_COLOR
help
This option supports common operations for
LP5521/5523/55231/5562/8501
devices.
@@ -408,8 +408,7 @@ config LEDS_LP55XX_COMMON
config LEDS_LP5521
tristate "LED Support for N.S. LP5521 LED driver chip"
depends on LEDS_CLASS && I2C
- depends on LEDS_CLASS_MULTI_COLOR || !LEDS_CLASS_MULTI_COLOR
- select LEDS_LP55XX_COMMON
+ depends on LEDS_LP55XX_COMMON
help
If you say yes here you get support for the National
Semiconductor
LP5521 LED driver. It is 3 channel chip with programmable
engines.
@@ -419,8 +418,7 @@ config LEDS_LP5521
config LEDS_LP5523
tristate "LED Support for TI/National LP5523/55231 LED driver chip"
depends on LEDS_CLASS && I2C
- depends on LEDS_CLASS_MULTI_COLOR || !LEDS_CLASS_MULTI_COLOR
- select LEDS_LP55XX_COMMON
+ depends on LEDS_LP55XX_COMMON
help
If you say yes here you get support for TI/National Semiconductor
LP5523/55231 LED driver.
@@ -431,7 +429,7 @@ config LEDS_LP5523
config LEDS_LP5562
tristate "LED Support for TI LP5562 LED driver chip"
depends on LEDS_CLASS && I2C
- select LEDS_LP55XX_COMMON
+ depends on LEDS_LP55XX_COMMON
help
If you say yes here you get support for TI LP5562 LED driver.
It is 4 channels chip with programmable engines.
@@ -441,7 +439,7 @@ config LEDS_LP5562
config LEDS_LP8501
tristate "LED Support for TI LP8501 LED driver chip"
depends on LEDS_CLASS && I2C
- select LEDS_LP55XX_COMMON
+ depends on LEDS_LP55XX_COMMON
help
If you say yes here you get support for TI LP8501 LED driver.
It is 9 channel chip with programmable engines.
--
Best regards,
Jacek Anaszewski
