On 10/30/20 9:51 AM, Gene Chen wrote:
Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> 於 2020年10月28日 週三 上午1:28寫道：
On 10/27/20 10:28 AM, Gene Chen wrote:
Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> 於 2020年10月21日 週三 上午5:47寫道：
On 10/20/20 8:44 AM, Gene Chen wrote:
Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> 於 2020年10月9日 週五 上午5:51寫道：
Hi Gene,
On 10/7/20 3:42 AM, Gene Chen wrote:
From: Gene Chen <gene_chen@xxxxxxxxxxx>
Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode,
3-channel RGB LED support Register/Flash/Breath Mode, and 1-channel for
moonlight LED.
Signed-off-by: Gene Chen <gene_chen@xxxxxxxxxxx>
---
drivers/leds/Kconfig | 12 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-mt6360.c | 783 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 796 insertions(+)
create mode 100644 drivers/leds/leds-mt6360.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1c181df..c7192dd 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -271,6 +271,18 @@ config LEDS_MT6323
This option enables support for on-chip LED drivers found on
Mediatek MT6323 PMIC.
+config LEDS_MT6360
+ tristate "LED Support for Mediatek MT6360 PMIC"
+ depends on LEDS_CLASS_FLASH && OF
+ depends on LEDS_CLASS_MULTICOLOR
Since CONFIG_LED_CLASS_MULTICOLOR can be turned off you need to have
below instead:
depends on LEDS_CLASS_MULTICOLOR || !!LEDS_CLASS_MULTICOLOR
My typo here, should be one "!":
depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
And you should also have
depends on LEDS_CLASS_FLASH || !LEDS_CLASS_FLASH
But to make it work correctly you would have to add registration
stubs to include/linux/led-class-flash.h similarly to LED mc stubs
in include/linux/led-class-multicolor.h.
Unless you want to prevent enabling the driver without RGB LED,
but that does not seem to be reasonable at first glance.
May I change to "select LEDS_CLASS_MULTICOLOR"?
I suppose RGB always use multicolor mode.
You will also have moonlight LED that will not need multicolor
framework. Is it somehow troublesome to keep "depends on"?
If only use ML LED and FLED, DTSI will only define ML LED and FLED.
Therefore, the drivers probe will not register rgb multicolor device.
Please test your use case again with my fixed "depends on".
In case when there is only ML LED and FLED in the DT it should
register both devices if LEDS_CLASS_FLASH is turned on.
Multicolor framework has nothing to do in this case.
But if you additionally had MC LED node, then it should
be registered only if LEDS_CLASS_MULTICOLOR is enabled.
Similarly, when FLED node is present, but LEDS_CLASS_FLASH
is off, and LEDS_CLASS_MULTICOLOR is on, the driver should still
compile, but register only LED MC device (if its node is present).
I think this case only register LED device, not LED "MC" device.
Because our FLASH is not a multicolor device.
No, here I was describing following setup:
- DT FLED node is present, CONFIG_LEDS_CLASS_FLASH is off
- DT MC node is present, CONFIG_LEDS_CLASS_MULTICOLOR is on
ML LED presence in DT is irrelevant in this case.
It should be always registered if there is corresponding DT node
and LEDS_CLASS is on.
Possible should be also the case when both LEDS_CLASS_FLASH
and LEDS_CLASS_MULTICOLOR are off. Then only LED class device
for ML LED will be registered (provided there is ML DT node).
But to make it possible you should have also "depends on LEDS_CLASS"
in the Kconfig entry.
According to your suggestion,
depends on LED_CLASS && LEDS_CLASS_FLASH && OF
s/LED_CLASS/LEDS_CLASS/
And you have to remove LEDS_CLASS_FLASH from above line.
depends on LEDS_CLASS_MULTICOLOR || !!LEDS_CLASS_MULTICOLOR
s/!!LEDS_CLASS_MULTICOLOR/!LEDS_CLASS_MULTICOLOR/
depends on LEDS_CLASS_FLASH || !LEDS_CLASS_FLASH
depends on MFD_MT6360
You will need V4L2_FLASH_LED_CLASS dependency as well, to avoid
build break, when it is set to 'm'.
To recap, following block of dependencies is required:
depends on LEDS_CLASS && OF
depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
depends on LEDS_CLASS_FLASH || !LEDS_CLASS_FLASH
depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
depends on MFD_MT6360
and source code add constraint
#if IS_ENABLED(CONFIG_LEDS_CLASS_MULTICOLOR)
ret = devm_led_classdev_multicolor_register_ext(parent, &led->rgb,
init_data);
#endif
#if IS_ENABLED(CONFIG_LEDS_CLASS_FLASH)
ret = devm_led_classdev_flash_register_ext(parent, &led->flash, init_data);
#endif
No, the guards should be in headers. That's why I recommended adding
no ops for LED flash class registration functions in previous email.
Please compare include/linux/led-class-multicolor.h and do similar
changes in include/linux/led-class-flash.h.
=============
Or Should I seperate two drivers?
one for RGB LED, one for ML LED and FLED
This would incur unnecessary code duplication.
--
Best regards,
Jacek Anaszewski
