| Message ID | 20240620210401.22053-1-ansuelsmth@gmail.com |
|---|---|
| Headers | show |
| Series | leds: leds-lp55xx: overhaul driver | expand |
On Thu, 20 Jun 2024, Christian Marangi wrote: > This long series is (as requested) a big overhaul of the lp55xx based > LED driver. > > As notice for these kind of LED chip there was the bad habit of copy > the old driver and just modify it enough to make it work with the new > model. Till v4 I was also doing the same by following the pattern and > the code format of previous driver. > > Since Lee didn't like this, here is the BIG series that generalize > pretty much anything in the 4 model currently supported. > > Indeed, although the LED chip have fundamental difference (page > support), things can be generalized and produce slimmer drivers by > putting everything in the lp55xx-common shared module. > > This result in the new model lp5569 being very small with only the > selftest portion to be custom. > > Lee also wasn't clear by the meaning of ENGINE in these LED driver, > so here some simple explaination. This is very common on these TI LED > chip. The ENGINE (there are always 3) is just some kind of processor > that execute a program (precompiled code ASM like) loaded in the SRAM. > Sysfs is used to load the pattern, and to start and stop the engine. > > These pattern can do all kind of complex thing with LEDs. Old LED chip > had 32bytes of space for the pattern but newer one (like lp5569) have > pages and if correctly configured can have massive pattern. > These pattern can do all kind of magic like loops that make the LED > pulse, change color and all kind of stuff. > > (For Lee, sorry if you will have to repeat some review that I might > have missed in the lp5569 driver) > > Changes v7: > - Add Suggested-by tag > - Fix checkpatch error for complex macro (rework define) > - Add missing values for fader conversion > - Align some function with redundant new line > - Capitalize every commit title > Changes v6: > - Fix compilation warning for ret unused in read_poll_timeout > (no idea why this is flagged only on some particular arch...) > - Fix missing bitfield.h in lp55x-common.c (again it seems this > header gets included in the flow if the arch use them or not...) > Changes v5: > - Big generalization patch > - Rework lp5569 driver with new generalized functions > - Drop all copyright header in lp5569 as the driver got reworked > entirely and it's not based on previous one anymore. > Changes v4: > - Fix reported buffer overflow due to a copypaste error > - Add comments to describe fw size logic > Changes v3: > - Add ACK tag to DT patch > - Enlarge and support program size up to 128bytes > Changes v2: > - Add ACK tag to DT patch > - Fix compilation error with target that doesn't > include bitfield.h > > Christian Marangi (20): > dt-bindings: leds-lp55xx: Limit pwr-sel property to ti,lp8501 > dt-bindings: leds-lp55xx: Add new ti,lp5569 compatible > leds: leds-lp55xx: Generalize stop_all_engine OP > leds: leds-lp55xx: Generalize probe/remove functions > leds: leds-lp55xx: Generalize load_engine function > leds: leds-lp55xx: Generalize load_engine_and_select_page function > leds: leds-lp55xx: Generalize run_engine function > leds: leds-lp55xx: Generalize update_program_memory function > leds: leds-lp55xx: Generalize firmware_loaded function > leds: leds-lp55xx: Generalize led_brightness function > leds: leds-lp55xx: Generalize multicolor_brightness function > leds: leds-lp55xx: Generalize set_led_current function > leds: leds-lp55xx: Generalize turn_off_channels function > leds: leds-lp55xx: Generalize stop_engine function > leds: leds-lp55xx: Generalize sysfs engine_load and engine_mode > leds: leds-lp55xx: Generalize sysfs engine_leds > leds: leds-lp55xx: Generalize sysfs master_fader > leds: leds-lp55xx: Support ENGINE program up to 128 bytes > leds: leds-lp55xx: Drop deprecated defines > leds: leds-lp5569: Add support for Texas Instruments LP5569 > > .../devicetree/bindings/leds/leds-lp55xx.yaml | 11 + > drivers/leds/Kconfig | 16 +- > drivers/leds/Makefile | 1 + > drivers/leds/leds-lp5521.c | 405 +--------- > drivers/leds/leds-lp5523.c | 734 ++--------------- > drivers/leds/leds-lp5562.c | 261 +----- > drivers/leds/leds-lp5569.c | 544 +++++++++++++ > drivers/leds/leds-lp55xx-common.c | 743 +++++++++++++++++- > drivers/leds/leds-lp55xx-common.h | 163 ++-- > drivers/leds/leds-lp8501.c | 313 +------- > 10 files changed, 1540 insertions(+), 1651 deletions(-) > create mode 100644 drivers/leds/leds-lp5569.c For whatever reason, this no longer applies. Please rebase it on Linux -next or for-leds-next and resubmit.
On Wed, Jun 26, 2024 at 04:42:55PM +0100, Lee Jones wrote: > On Thu, 20 Jun 2024, Christian Marangi wrote: > > > This long series is (as requested) a big overhaul of the lp55xx based > > LED driver. > > > > As notice for these kind of LED chip there was the bad habit of copy > > the old driver and just modify it enough to make it work with the new > > model. Till v4 I was also doing the same by following the pattern and > > the code format of previous driver. > > > > Since Lee didn't like this, here is the BIG series that generalize > > pretty much anything in the 4 model currently supported. > > > > Indeed, although the LED chip have fundamental difference (page > > support), things can be generalized and produce slimmer drivers by > > putting everything in the lp55xx-common shared module. > > > > This result in the new model lp5569 being very small with only the > > selftest portion to be custom. > > > > Lee also wasn't clear by the meaning of ENGINE in these LED driver, > > so here some simple explaination. This is very common on these TI LED > > chip. The ENGINE (there are always 3) is just some kind of processor > > that execute a program (precompiled code ASM like) loaded in the SRAM. > > Sysfs is used to load the pattern, and to start and stop the engine. > > > > These pattern can do all kind of complex thing with LEDs. Old LED chip > > had 32bytes of space for the pattern but newer one (like lp5569) have > > pages and if correctly configured can have massive pattern. > > These pattern can do all kind of magic like loops that make the LED > > pulse, change color and all kind of stuff. > > > > (For Lee, sorry if you will have to repeat some review that I might > > have missed in the lp5569 driver) > > > > Changes v7: > > - Add Suggested-by tag > > - Fix checkpatch error for complex macro (rework define) > > - Add missing values for fader conversion > > - Align some function with redundant new line > > - Capitalize every commit title > > Changes v6: > > - Fix compilation warning for ret unused in read_poll_timeout > > (no idea why this is flagged only on some particular arch...) > > - Fix missing bitfield.h in lp55x-common.c (again it seems this > > header gets included in the flow if the arch use them or not...) > > Changes v5: > > - Big generalization patch > > - Rework lp5569 driver with new generalized functions > > - Drop all copyright header in lp5569 as the driver got reworked > > entirely and it's not based on previous one anymore. > > Changes v4: > > - Fix reported buffer overflow due to a copypaste error > > - Add comments to describe fw size logic > > Changes v3: > > - Add ACK tag to DT patch > > - Enlarge and support program size up to 128bytes > > Changes v2: > > - Add ACK tag to DT patch > > - Fix compilation error with target that doesn't > > include bitfield.h > > > > Christian Marangi (20): > > dt-bindings: leds-lp55xx: Limit pwr-sel property to ti,lp8501 > > dt-bindings: leds-lp55xx: Add new ti,lp5569 compatible > > leds: leds-lp55xx: Generalize stop_all_engine OP > > leds: leds-lp55xx: Generalize probe/remove functions > > leds: leds-lp55xx: Generalize load_engine function > > leds: leds-lp55xx: Generalize load_engine_and_select_page function > > leds: leds-lp55xx: Generalize run_engine function > > leds: leds-lp55xx: Generalize update_program_memory function > > leds: leds-lp55xx: Generalize firmware_loaded function > > leds: leds-lp55xx: Generalize led_brightness function > > leds: leds-lp55xx: Generalize multicolor_brightness function > > leds: leds-lp55xx: Generalize set_led_current function > > leds: leds-lp55xx: Generalize turn_off_channels function > > leds: leds-lp55xx: Generalize stop_engine function > > leds: leds-lp55xx: Generalize sysfs engine_load and engine_mode > > leds: leds-lp55xx: Generalize sysfs engine_leds > > leds: leds-lp55xx: Generalize sysfs master_fader > > leds: leds-lp55xx: Support ENGINE program up to 128 bytes > > leds: leds-lp55xx: Drop deprecated defines > > leds: leds-lp5569: Add support for Texas Instruments LP5569 > > > > .../devicetree/bindings/leds/leds-lp55xx.yaml | 11 + > > drivers/leds/Kconfig | 16 +- > > drivers/leds/Makefile | 1 + > > drivers/leds/leds-lp5521.c | 405 +--------- > > drivers/leds/leds-lp5523.c | 734 ++--------------- > > drivers/leds/leds-lp5562.c | 261 +----- > > drivers/leds/leds-lp5569.c | 544 +++++++++++++ > > drivers/leds/leds-lp55xx-common.c | 743 +++++++++++++++++- > > drivers/leds/leds-lp55xx-common.h | 163 ++-- > > drivers/leds/leds-lp8501.c | 313 +------- > > 10 files changed, 1540 insertions(+), 1651 deletions(-) > > create mode 100644 drivers/leds/leds-lp5569.c > > For whatever reason, this no longer applies. > > Please rebase it on Linux -next or for-leds-next and resubmit. > Hi Lee., I just checked and yes there is to rebase the series due to c0e3d2beeb03 ("leds: Drop explicit initialization of struct i2c_device_id::driver_data to 0") But I notice that something went bad in reverting some commits from the v7 series and in for-leds-next there are still 3 commit to drop mainly - 3bed6364ffd0057bd10c081d2e24aa7e8555e0a8 - 3bcc6d6ad7821aa459826592f830d76a3dc6f79f - 5a402b40c651baf758ad884aed36c0ea70cd8b1e I'm sending v8 with everything rebased but please keep in mind that those commits needs to be reverted as the leds one also somehow got half applied (no idea what happen)
On Thu, 20 Jun 2024 23:03:16 +0200, Christian Marangi wrote: > This long series is (as requested) a big overhaul of the lp55xx based > LED driver. > > As notice for these kind of LED chip there was the bad habit of copy > the old driver and just modify it enough to make it work with the new > model. Till v4 I was also doing the same by following the pattern and > the code format of previous driver. > > [...] Applied, thanks! [01/20] dt-bindings: leds-lp55xx: Limit pwr-sel property to ti,lp8501 (no commit info) [02/20] dt-bindings: leds-lp55xx: Add new ti,lp5569 compatible commit: c4bd4feece4103388d15c99461741604fec0ebbc [03/20] leds: leds-lp55xx: Generalize stop_all_engine OP (no commit info) [04/20] leds: leds-lp55xx: Generalize probe/remove functions (no commit info) [05/20] leds: leds-lp55xx: Generalize load_engine function (no commit info) [06/20] leds: leds-lp55xx: Generalize load_engine_and_select_page function (no commit info) [07/20] leds: leds-lp55xx: Generalize run_engine function (no commit info) [08/20] leds: leds-lp55xx: Generalize update_program_memory function (no commit info) [09/20] leds: leds-lp55xx: Generalize firmware_loaded function (no commit info) [10/20] leds: leds-lp55xx: Generalize led_brightness function (no commit info) [11/20] leds: leds-lp55xx: Generalize multicolor_brightness function (no commit info) [12/20] leds: leds-lp55xx: Generalize set_led_current function (no commit info) [13/20] leds: leds-lp55xx: Generalize turn_off_channels function (no commit info) [14/20] leds: leds-lp55xx: Generalize stop_engine function (no commit info) [15/20] leds: leds-lp55xx: Generalize sysfs engine_load and engine_mode (no commit info) [16/20] leds: leds-lp55xx: Generalize sysfs engine_leds (no commit info) [17/20] leds: leds-lp55xx: Generalize sysfs master_fader (no commit info) [18/20] leds: leds-lp55xx: Support ENGINE program up to 128 bytes (no commit info) [19/20] leds: leds-lp55xx: Drop deprecated defines (no commit info) [20/20] leds: leds-lp5569: Add support for Texas Instruments LP5569 (no commit info) -- Lee Jones [李琼斯]
On Wed, 26 Jun 2024, Christian Marangi wrote: > On Wed, Jun 26, 2024 at 04:42:55PM +0100, Lee Jones wrote: > > On Thu, 20 Jun 2024, Christian Marangi wrote: > > > > > This long series is (as requested) a big overhaul of the lp55xx based > > > LED driver. > > > > > > As notice for these kind of LED chip there was the bad habit of copy > > > the old driver and just modify it enough to make it work with the new > > > model. Till v4 I was also doing the same by following the pattern and > > > the code format of previous driver. > > > > > > Since Lee didn't like this, here is the BIG series that generalize > > > pretty much anything in the 4 model currently supported. > > > > > > Indeed, although the LED chip have fundamental difference (page > > > support), things can be generalized and produce slimmer drivers by > > > putting everything in the lp55xx-common shared module. > > > > > > This result in the new model lp5569 being very small with only the > > > selftest portion to be custom. > > > > > > Lee also wasn't clear by the meaning of ENGINE in these LED driver, > > > so here some simple explaination. This is very common on these TI LED > > > chip. The ENGINE (there are always 3) is just some kind of processor > > > that execute a program (precompiled code ASM like) loaded in the SRAM. > > > Sysfs is used to load the pattern, and to start and stop the engine. > > > > > > These pattern can do all kind of complex thing with LEDs. Old LED chip > > > had 32bytes of space for the pattern but newer one (like lp5569) have > > > pages and if correctly configured can have massive pattern. > > > These pattern can do all kind of magic like loops that make the LED > > > pulse, change color and all kind of stuff. > > > > > > (For Lee, sorry if you will have to repeat some review that I might > > > have missed in the lp5569 driver) > > > > > > Changes v7: > > > - Add Suggested-by tag > > > - Fix checkpatch error for complex macro (rework define) > > > - Add missing values for fader conversion > > > - Align some function with redundant new line > > > - Capitalize every commit title > > > Changes v6: > > > - Fix compilation warning for ret unused in read_poll_timeout > > > (no idea why this is flagged only on some particular arch...) > > > - Fix missing bitfield.h in lp55x-common.c (again it seems this > > > header gets included in the flow if the arch use them or not...) > > > Changes v5: > > > - Big generalization patch > > > - Rework lp5569 driver with new generalized functions > > > - Drop all copyright header in lp5569 as the driver got reworked > > > entirely and it's not based on previous one anymore. > > > Changes v4: > > > - Fix reported buffer overflow due to a copypaste error > > > - Add comments to describe fw size logic > > > Changes v3: > > > - Add ACK tag to DT patch > > > - Enlarge and support program size up to 128bytes > > > Changes v2: > > > - Add ACK tag to DT patch > > > - Fix compilation error with target that doesn't > > > include bitfield.h > > > > > > Christian Marangi (20): > > > dt-bindings: leds-lp55xx: Limit pwr-sel property to ti,lp8501 > > > dt-bindings: leds-lp55xx: Add new ti,lp5569 compatible > > > leds: leds-lp55xx: Generalize stop_all_engine OP > > > leds: leds-lp55xx: Generalize probe/remove functions > > > leds: leds-lp55xx: Generalize load_engine function > > > leds: leds-lp55xx: Generalize load_engine_and_select_page function > > > leds: leds-lp55xx: Generalize run_engine function > > > leds: leds-lp55xx: Generalize update_program_memory function > > > leds: leds-lp55xx: Generalize firmware_loaded function > > > leds: leds-lp55xx: Generalize led_brightness function > > > leds: leds-lp55xx: Generalize multicolor_brightness function > > > leds: leds-lp55xx: Generalize set_led_current function > > > leds: leds-lp55xx: Generalize turn_off_channels function > > > leds: leds-lp55xx: Generalize stop_engine function > > > leds: leds-lp55xx: Generalize sysfs engine_load and engine_mode > > > leds: leds-lp55xx: Generalize sysfs engine_leds > > > leds: leds-lp55xx: Generalize sysfs master_fader > > > leds: leds-lp55xx: Support ENGINE program up to 128 bytes > > > leds: leds-lp55xx: Drop deprecated defines > > > leds: leds-lp5569: Add support for Texas Instruments LP5569 > > > > > > .../devicetree/bindings/leds/leds-lp55xx.yaml | 11 + > > > drivers/leds/Kconfig | 16 +- > > > drivers/leds/Makefile | 1 + > > > drivers/leds/leds-lp5521.c | 405 +--------- > > > drivers/leds/leds-lp5523.c | 734 ++--------------- > > > drivers/leds/leds-lp5562.c | 261 +----- > > > drivers/leds/leds-lp5569.c | 544 +++++++++++++ > > > drivers/leds/leds-lp55xx-common.c | 743 +++++++++++++++++- > > > drivers/leds/leds-lp55xx-common.h | 163 ++-- > > > drivers/leds/leds-lp8501.c | 313 +------- > > > 10 files changed, 1540 insertions(+), 1651 deletions(-) > > > create mode 100644 drivers/leds/leds-lp5569.c > > > > For whatever reason, this no longer applies. > > > > Please rebase it on Linux -next or for-leds-next and resubmit. > > > > Hi Lee., > > I just checked and yes there is to rebase the series due to > c0e3d2beeb03 ("leds: Drop explicit initialization of struct > i2c_device_id::driver_data to 0") > > But I notice that something went bad in reverting some commits from the > v7 series and in for-leds-next there are still 3 commit to drop > > mainly > - 3bed6364ffd0057bd10c081d2e24aa7e8555e0a8 > - 3bcc6d6ad7821aa459826592f830d76a3dc6f79f > - 5a402b40c651baf758ad884aed36c0ea70cd8b1e > > I'm sending v8 with everything rebased but please keep in mind that > those commits needs to be reverted as the leds one also somehow got half > applied (no idea what happen) This must be related to the tooling error that happened recently. I have removed those 3 patches from the tree and re-pushed the branch. If you need to rebase again for-leds-next is now up-to-date.
On Thu, 20 Jun 2024 23:03:16 +0200, Christian Marangi wrote: > This long series is (as requested) a big overhaul of the lp55xx based > LED driver. > > As notice for these kind of LED chip there was the bad habit of copy > the old driver and just modify it enough to make it work with the new > model. Till v4 I was also doing the same by following the pattern and > the code format of previous driver. > > [...] Applied, thanks! [01/20] dt-bindings: leds-lp55xx: Limit pwr-sel property to ti,lp8501 commit: 468434a059a7d1fad4b98c2ca080817b1520cbdc [02/20] dt-bindings: leds-lp55xx: Add new ti,lp5569 compatible commit: a6ca48430de6e87644203bdca03f4065f5b9df7a [03/20] leds: leds-lp55xx: Generalize stop_all_engine OP commit: a9b202b9cf0e817be756a720920ad4b522e6f6aa [04/20] leds: leds-lp55xx: Generalize probe/remove functions commit: db30c2891bfc74acb8823edee5f39cbc36bd9a4d [05/20] leds: leds-lp55xx: Generalize load_engine function commit: 4d310b96f2db602830c40f82a75ede799b243cce [06/20] leds: leds-lp55xx: Generalize load_engine_and_select_page function commit: 409a9dc53682b9f02793584d17721ab3e1b9c86f [07/20] leds: leds-lp55xx: Generalize run_engine function commit: 42a9eaac9784e9b3df56f1947526d7d4d0ed9b26 [08/20] leds: leds-lp55xx: Generalize update_program_memory function commit: 31379a57cf2f155eb147ace86547b7143592945a [09/20] leds: leds-lp55xx: Generalize firmware_loaded function commit: a3df1906fb9aa9ff45149e0a3c6434b2cef4f6e7 [10/20] leds: leds-lp55xx: Generalize led_brightness function commit: c63580b27a2c638cbae2fc26484b0bf29f303134 [11/20] leds: leds-lp55xx: Generalize multicolor_brightness function commit: 794826b2d87538a0fa5429957439f82bb7f32b53 [12/20] leds: leds-lp55xx: Generalize set_led_current function commit: 01e0290d17b2fb9717ee80fed512b32e0460b14c [13/20] leds: leds-lp55xx: Generalize turn_off_channels function commit: e35bc5d8a023a55a5f895d6648a455ed83dc0db2 [14/20] leds: leds-lp55xx: Generalize stop_engine function commit: 43e91e5eb9c8b36ddd1dc239e0d8c36cc034e8ca [15/20] leds: leds-lp55xx: Generalize sysfs engine_load and engine_mode commit: 082a4d3f068734eb242e38892d0977ef271c0143 [16/20] leds: leds-lp55xx: Generalize sysfs engine_leds commit: 8913c2c14728851f110e0d439d5bb2360c767cd2 [17/20] leds: leds-lp55xx: Generalize sysfs master_fader commit: 5a15b2ab57095a7c8597d42efbfe452844578785 [18/20] leds: leds-lp55xx: Support ENGINE program up to 128 bytes commit: b9d55087dfa950aecece1cf864d3918a12694c25 [19/20] leds: leds-lp55xx: Drop deprecated defines commit: 49d943a426d1e2c034ff2f132f65590dbdc01fbd [20/20] leds: leds-lp5569: Add support for Texas Instruments LP5569 commit: 30c6743cc89cdb357d1f8a98978da0f7c138130b -- Lee Jones [李琼斯]
On Wed, 26 Jun 2024, Lee Jones wrote: > On Wed, 26 Jun 2024, Christian Marangi wrote: > > > On Wed, Jun 26, 2024 at 04:42:55PM +0100, Lee Jones wrote: > > > On Thu, 20 Jun 2024, Christian Marangi wrote: > > > > > > > This long series is (as requested) a big overhaul of the lp55xx based > > > > LED driver. > > > > > > > > As notice for these kind of LED chip there was the bad habit of copy > > > > the old driver and just modify it enough to make it work with the new > > > > model. Till v4 I was also doing the same by following the pattern and > > > > the code format of previous driver. > > > > > > > > Since Lee didn't like this, here is the BIG series that generalize > > > > pretty much anything in the 4 model currently supported. > > > > > > > > Indeed, although the LED chip have fundamental difference (page > > > > support), things can be generalized and produce slimmer drivers by > > > > putting everything in the lp55xx-common shared module. > > > > > > > > This result in the new model lp5569 being very small with only the > > > > selftest portion to be custom. > > > > > > > > Lee also wasn't clear by the meaning of ENGINE in these LED driver, > > > > so here some simple explaination. This is very common on these TI LED > > > > chip. The ENGINE (there are always 3) is just some kind of processor > > > > that execute a program (precompiled code ASM like) loaded in the SRAM. > > > > Sysfs is used to load the pattern, and to start and stop the engine. > > > > > > > > These pattern can do all kind of complex thing with LEDs. Old LED chip > > > > had 32bytes of space for the pattern but newer one (like lp5569) have > > > > pages and if correctly configured can have massive pattern. > > > > These pattern can do all kind of magic like loops that make the LED > > > > pulse, change color and all kind of stuff. > > > > > > > > (For Lee, sorry if you will have to repeat some review that I might > > > > have missed in the lp5569 driver) > > > > > > > > Changes v7: > > > > - Add Suggested-by tag > > > > - Fix checkpatch error for complex macro (rework define) > > > > - Add missing values for fader conversion > > > > - Align some function with redundant new line > > > > - Capitalize every commit title > > > > Changes v6: > > > > - Fix compilation warning for ret unused in read_poll_timeout > > > > (no idea why this is flagged only on some particular arch...) > > > > - Fix missing bitfield.h in lp55x-common.c (again it seems this > > > > header gets included in the flow if the arch use them or not...) > > > > Changes v5: > > > > - Big generalization patch > > > > - Rework lp5569 driver with new generalized functions > > > > - Drop all copyright header in lp5569 as the driver got reworked > > > > entirely and it's not based on previous one anymore. > > > > Changes v4: > > > > - Fix reported buffer overflow due to a copypaste error > > > > - Add comments to describe fw size logic > > > > Changes v3: > > > > - Add ACK tag to DT patch > > > > - Enlarge and support program size up to 128bytes > > > > Changes v2: > > > > - Add ACK tag to DT patch > > > > - Fix compilation error with target that doesn't > > > > include bitfield.h > > > > > > > > Christian Marangi (20): > > > > dt-bindings: leds-lp55xx: Limit pwr-sel property to ti,lp8501 > > > > dt-bindings: leds-lp55xx: Add new ti,lp5569 compatible > > > > leds: leds-lp55xx: Generalize stop_all_engine OP > > > > leds: leds-lp55xx: Generalize probe/remove functions > > > > leds: leds-lp55xx: Generalize load_engine function > > > > leds: leds-lp55xx: Generalize load_engine_and_select_page function > > > > leds: leds-lp55xx: Generalize run_engine function > > > > leds: leds-lp55xx: Generalize update_program_memory function > > > > leds: leds-lp55xx: Generalize firmware_loaded function > > > > leds: leds-lp55xx: Generalize led_brightness function > > > > leds: leds-lp55xx: Generalize multicolor_brightness function > > > > leds: leds-lp55xx: Generalize set_led_current function > > > > leds: leds-lp55xx: Generalize turn_off_channels function > > > > leds: leds-lp55xx: Generalize stop_engine function > > > > leds: leds-lp55xx: Generalize sysfs engine_load and engine_mode > > > > leds: leds-lp55xx: Generalize sysfs engine_leds > > > > leds: leds-lp55xx: Generalize sysfs master_fader > > > > leds: leds-lp55xx: Support ENGINE program up to 128 bytes > > > > leds: leds-lp55xx: Drop deprecated defines > > > > leds: leds-lp5569: Add support for Texas Instruments LP5569 > > > > > > > > .../devicetree/bindings/leds/leds-lp55xx.yaml | 11 + > > > > drivers/leds/Kconfig | 16 +- > > > > drivers/leds/Makefile | 1 + > > > > drivers/leds/leds-lp5521.c | 405 +--------- > > > > drivers/leds/leds-lp5523.c | 734 ++--------------- > > > > drivers/leds/leds-lp5562.c | 261 +----- > > > > drivers/leds/leds-lp5569.c | 544 +++++++++++++ > > > > drivers/leds/leds-lp55xx-common.c | 743 +++++++++++++++++- > > > > drivers/leds/leds-lp55xx-common.h | 163 ++-- > > > > drivers/leds/leds-lp8501.c | 313 +------- > > > > 10 files changed, 1540 insertions(+), 1651 deletions(-) > > > > create mode 100644 drivers/leds/leds-lp5569.c > > > > > > For whatever reason, this no longer applies. > > > > > > Please rebase it on Linux -next or for-leds-next and resubmit. > > > > > > > Hi Lee., > > > > I just checked and yes there is to rebase the series due to > > c0e3d2beeb03 ("leds: Drop explicit initialization of struct > > i2c_device_id::driver_data to 0") > > > > But I notice that something went bad in reverting some commits from the > > v7 series and in for-leds-next there are still 3 commit to drop > > > > mainly > > - 3bed6364ffd0057bd10c081d2e24aa7e8555e0a8 > > - 3bcc6d6ad7821aa459826592f830d76a3dc6f79f > > - 5a402b40c651baf758ad884aed36c0ea70cd8b1e > > > > I'm sending v8 with everything rebased but please keep in mind that > > those commits needs to be reverted as the leds one also somehow got half > > applied (no idea what happen) > > This must be related to the tooling error that happened recently. > > I have removed those 3 patches from the tree and re-pushed the branch. > > If you need to rebase again for-leds-next is now up-to-date. Okay v8 applied without issues. Thank you.
On Wed, Jun 26, 2024 at 05:09:26PM +0100, Lee Jones wrote: > On Wed, 26 Jun 2024, Lee Jones wrote: > > > On Wed, 26 Jun 2024, Christian Marangi wrote: > > > > > On Wed, Jun 26, 2024 at 04:42:55PM +0100, Lee Jones wrote: > > > > On Thu, 20 Jun 2024, Christian Marangi wrote: > > > > > > > > > This long series is (as requested) a big overhaul of the lp55xx based > > > > > LED driver. > > > > > > > > > > As notice for these kind of LED chip there was the bad habit of copy > > > > > the old driver and just modify it enough to make it work with the new > > > > > model. Till v4 I was also doing the same by following the pattern and > > > > > the code format of previous driver. > > > > > > > > > > Since Lee didn't like this, here is the BIG series that generalize > > > > > pretty much anything in the 4 model currently supported. > > > > > > > > > > Indeed, although the LED chip have fundamental difference (page > > > > > support), things can be generalized and produce slimmer drivers by > > > > > putting everything in the lp55xx-common shared module. > > > > > > > > > > This result in the new model lp5569 being very small with only the > > > > > selftest portion to be custom. > > > > > > > > > > Lee also wasn't clear by the meaning of ENGINE in these LED driver, > > > > > so here some simple explaination. This is very common on these TI LED > > > > > chip. The ENGINE (there are always 3) is just some kind of processor > > > > > that execute a program (precompiled code ASM like) loaded in the SRAM. > > > > > Sysfs is used to load the pattern, and to start and stop the engine. > > > > > > > > > > These pattern can do all kind of complex thing with LEDs. Old LED chip > > > > > had 32bytes of space for the pattern but newer one (like lp5569) have > > > > > pages and if correctly configured can have massive pattern. > > > > > These pattern can do all kind of magic like loops that make the LED > > > > > pulse, change color and all kind of stuff. > > > > > > > > > > (For Lee, sorry if you will have to repeat some review that I might > > > > > have missed in the lp5569 driver) > > > > > > > > > > Changes v7: > > > > > - Add Suggested-by tag > > > > > - Fix checkpatch error for complex macro (rework define) > > > > > - Add missing values for fader conversion > > > > > - Align some function with redundant new line > > > > > - Capitalize every commit title > > > > > Changes v6: > > > > > - Fix compilation warning for ret unused in read_poll_timeout > > > > > (no idea why this is flagged only on some particular arch...) > > > > > - Fix missing bitfield.h in lp55x-common.c (again it seems this > > > > > header gets included in the flow if the arch use them or not...) > > > > > Changes v5: > > > > > - Big generalization patch > > > > > - Rework lp5569 driver with new generalized functions > > > > > - Drop all copyright header in lp5569 as the driver got reworked > > > > > entirely and it's not based on previous one anymore. > > > > > Changes v4: > > > > > - Fix reported buffer overflow due to a copypaste error > > > > > - Add comments to describe fw size logic > > > > > Changes v3: > > > > > - Add ACK tag to DT patch > > > > > - Enlarge and support program size up to 128bytes > > > > > Changes v2: > > > > > - Add ACK tag to DT patch > > > > > - Fix compilation error with target that doesn't > > > > > include bitfield.h > > > > > > > > > > Christian Marangi (20): > > > > > dt-bindings: leds-lp55xx: Limit pwr-sel property to ti,lp8501 > > > > > dt-bindings: leds-lp55xx: Add new ti,lp5569 compatible > > > > > leds: leds-lp55xx: Generalize stop_all_engine OP > > > > > leds: leds-lp55xx: Generalize probe/remove functions > > > > > leds: leds-lp55xx: Generalize load_engine function > > > > > leds: leds-lp55xx: Generalize load_engine_and_select_page function > > > > > leds: leds-lp55xx: Generalize run_engine function > > > > > leds: leds-lp55xx: Generalize update_program_memory function > > > > > leds: leds-lp55xx: Generalize firmware_loaded function > > > > > leds: leds-lp55xx: Generalize led_brightness function > > > > > leds: leds-lp55xx: Generalize multicolor_brightness function > > > > > leds: leds-lp55xx: Generalize set_led_current function > > > > > leds: leds-lp55xx: Generalize turn_off_channels function > > > > > leds: leds-lp55xx: Generalize stop_engine function > > > > > leds: leds-lp55xx: Generalize sysfs engine_load and engine_mode > > > > > leds: leds-lp55xx: Generalize sysfs engine_leds > > > > > leds: leds-lp55xx: Generalize sysfs master_fader > > > > > leds: leds-lp55xx: Support ENGINE program up to 128 bytes > > > > > leds: leds-lp55xx: Drop deprecated defines > > > > > leds: leds-lp5569: Add support for Texas Instruments LP5569 > > > > > > > > > > .../devicetree/bindings/leds/leds-lp55xx.yaml | 11 + > > > > > drivers/leds/Kconfig | 16 +- > > > > > drivers/leds/Makefile | 1 + > > > > > drivers/leds/leds-lp5521.c | 405 +--------- > > > > > drivers/leds/leds-lp5523.c | 734 ++--------------- > > > > > drivers/leds/leds-lp5562.c | 261 +----- > > > > > drivers/leds/leds-lp5569.c | 544 +++++++++++++ > > > > > drivers/leds/leds-lp55xx-common.c | 743 +++++++++++++++++- > > > > > drivers/leds/leds-lp55xx-common.h | 163 ++-- > > > > > drivers/leds/leds-lp8501.c | 313 +------- > > > > > 10 files changed, 1540 insertions(+), 1651 deletions(-) > > > > > create mode 100644 drivers/leds/leds-lp5569.c > > > > > > > > For whatever reason, this no longer applies. > > > > > > > > Please rebase it on Linux -next or for-leds-next and resubmit. > > > > > > > > > > Hi Lee., > > > > > > I just checked and yes there is to rebase the series due to > > > c0e3d2beeb03 ("leds: Drop explicit initialization of struct > > > i2c_device_id::driver_data to 0") > > > > > > But I notice that something went bad in reverting some commits from the > > > v7 series and in for-leds-next there are still 3 commit to drop > > > > > > mainly > > > - 3bed6364ffd0057bd10c081d2e24aa7e8555e0a8 > > > - 3bcc6d6ad7821aa459826592f830d76a3dc6f79f > > > - 5a402b40c651baf758ad884aed36c0ea70cd8b1e > > > > > > I'm sending v8 with everything rebased but please keep in mind that > > > those commits needs to be reverted as the leds one also somehow got half > > > applied (no idea what happen) > > > > This must be related to the tooling error that happened recently. > > > > I have removed those 3 patches from the tree and re-pushed the branch. > > > > If you need to rebase again for-leds-next is now up-to-date. > > Okay v8 applied without issues. > > Thank you. > Happy to have this finally merged and sorry for the big series and for the mess. Again thank to you for review and accept the changes!