| Message ID | 20230303095023.538917-1-quic_fenglinw@quicinc.com |
|---|---|
| Headers | show |
| Series | Add LED driver for flash module in QCOM PMICs | expand |
On Fri, 03 Mar 2023, Fenglin Wu wrote: > Add initial driver to support flash LED module found in Qualcomm > Technologies, Inc. PMICs. The flash module can have 3 or 4 channels > and each channel can be controlled indepedently and support full scale > current up to 1.5 A. It also supports connecting two channels together > to supply one LED component with full scale current up to 2 A. In that > case, the current will be split on each channel symmetrically and the > channels will be enabled and disabled at the same time. > > Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com> > Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sm7225-fairphone-fp4 + pm6150l > --- > drivers/leds/flash/Kconfig | 15 + > drivers/leds/flash/Makefile | 1 + > drivers/leds/flash/leds-qcom-flash.c | 773 +++++++++++++++++++++++++++ > 3 files changed, 789 insertions(+) > create mode 100644 drivers/leds/flash/leds-qcom-flash.c Applied, thanks -- Lee Jones [李琼斯]
Hi! > @@ -61,6 +61,21 @@ config LEDS_MT6360 > Independent current sources supply for each flash LED support torch > and strobe mode. > > +config LEDS_QCOM_FLASH > + tristate "LED support for flash module inside Qualcomm Technologies, Inc. PMIC" > + depends on MFD_SPMI_PMIC || COMPILE_TEST > + depends on LEDS_CLASS && OF > + depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS > + select REGMAP > + help > + This option enables support for the flash module found in Qualcomm > + Technologies, Inc. PMICs. The flash module can have 3 or 4 flash LED > + channels and each channel is programmable to support up to 1.5 A full > + scale current. It also supports connecting two channels' output together > + to supply one LED component to achieve current up to 2 A. In such case, > + the total LED current will be split symmetrically on each channel and > + they will be enabled/disabled at the same time. "This can be built as a module, module will be called..." > +static int qcom_flash_led_brightness_set(struct led_classdev *led_cdev, > + enum led_brightness brightness) > +{ > + struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev); > + struct qcom_flash_led *led = flcdev_to_qcom_fled(fled_cdev); > + u32 current_ma = brightness * led->max_torch_current_ma / LED_FULL; > + bool enable = !!brightness; > + int rc; > + > + rc = set_flash_current(led, current_ma, TORCH_MODE); > + if (rc) > + return rc; I'd not mind supporting more than one brightness level -- AFAICT hw can do that. BR, Pavel
Hi! > Add initial driver to support flash LED module found in Qualcomm > Technologies, Inc. PMICs. The flash module can have 3 or 4 channels > and each channel can be controlled indepedently and support full scale > current up to 1.5 A. It also supports connecting two channels together > to supply one LED component with full scale current up to 2 A. In that > case, the current will be split on each channel symmetrically and the > channels will be enabled and disabled at the same time. > +static int qcom_flash_strobe_set(struct led_classdev_flash *fled_cdev, bool state) > +{ > + struct qcom_flash_led *led = flcdev_to_qcom_fled(fled_cdev); > + int rc; > + > + rc = set_flash_current(led, led->flash_current_ma, FLASH_MODE); > + if (rc) > + return rc; > + > + rc = set_flash_timeout(led, led->flash_timeout_ms); > + if (rc) > + return rc; > + > + rc = set_flash_module_en(led, state); > + if (rc) > + return rc; > + > + return set_flash_strobe(led, SW_STROBE, state); > +} Should we disable the module before setting the current? It might be already active due to torch mode... > + return -EINVAL; > + } > + > + flash_data->v4l2_flash = devm_kcalloc(dev, count, > + sizeof(*flash_data->v4l2_flash), GFP_KERNEL); > + if (!flash_data->v4l2_flash) > + return -ENOMEM; > + > + device_for_each_child_node(dev, child) { > + led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL); > + if (!led) { > + rc = -ENOMEM; > + goto release; > + } > + > + led->flash_data = flash_data; > + rc = qcom_flash_register_led_device(dev, child, led); > + if (rc < 0) > + goto release; > + > + flash_data->leds_count++; > + } Do you need to do of_node_put in error paths? BR, Pavel
On Fri 2023-03-03 17:50:22, Fenglin Wu wrote: > Add initial driver to support flash LED module found in Qualcomm > Technologies, Inc. PMICs. The flash module can have 3 or 4 channels > and each channel can be controlled indepedently and support full scale > current up to 1.5 A. It also supports connecting two channels together > to supply one LED component with full scale current up to 2 A. In that > case, the current will be split on each channel symmetrically and the > channels will be enabled and disabled at the same time. > > Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com> > Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sm7225-fairphone-fp4 + pm6150l > + flash->led_cdev.brightness_set_blocking = qcom_flash_led_brightness_set; > + > + init_data.fwnode = node; > + init_data.devicename = NULL; > + init_data.default_label = NULL; > + init_data.devname_mandatory = false; > + > + rc = devm_led_classdev_flash_register_ext(dev, flash, &init_data); > + if (rc < 0) { > + dev_err(dev, "Register flash LED classdev failed, rc=%d\n", rc); > + return rc; > + } I'd expect setting max_brightness somewhere around here. What does cat /sys/class/leds/*/max_brightness say (and what directory name are you using)? BR, Pavel
On 3/26/2023 1:09 AM, Pavel Machek wrote: > Hi! >> @@ -61,6 +61,21 @@ config LEDS_MT6360 >> Independent current sources supply for each flash LED support torch >> and strobe mode. >> >> +config LEDS_QCOM_FLASH >> + tristate "LED support for flash module inside Qualcomm Technologies, Inc. PMIC" >> + depends on MFD_SPMI_PMIC || COMPILE_TEST >> + depends on LEDS_CLASS && OF >> + depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS >> + select REGMAP >> + help >> + This option enables support for the flash module found in Qualcomm >> + Technologies, Inc. PMICs. The flash module can have 3 or 4 flash LED >> + channels and each channel is programmable to support up to 1.5 A full >> + scale current. It also supports connecting two channels' output together >> + to supply one LED component to achieve current up to 2 A. In such case, >> + the total LED current will be split symmetrically on each channel and >> + they will be enabled/disabled at the same time. > > "This can be built as a module, module will be called..." I can update it in a new change considering the initial change has been applied. > >> +static int qcom_flash_led_brightness_set(struct led_classdev *led_cdev, >> + enum led_brightness brightness) >> +{ >> + struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev); >> + struct qcom_flash_led *led = flcdev_to_qcom_fled(fled_cdev); >> + u32 current_ma = brightness * led->max_torch_current_ma / LED_FULL; >> + bool enable = !!brightness; >> + int rc; >> + >> + rc = set_flash_current(led, current_ma, TORCH_MODE); >> + if (rc) >> + return rc; > > I'd not mind supporting more than one brightness level -- AFAICT hw can do that. It does support multiple levels and the 'current_ma' is calculated based on different brightness level. > > BR, > Pavel
On 3/26/2023 1:33 AM, Pavel Machek wrote: > Hi! > >> Add initial driver to support flash LED module found in Qualcomm >> Technologies, Inc. PMICs. The flash module can have 3 or 4 channels >> and each channel can be controlled indepedently and support full scale >> current up to 1.5 A. It also supports connecting two channels together >> to supply one LED component with full scale current up to 2 A. In that >> case, the current will be split on each channel symmetrically and the >> channels will be enabled and disabled at the same time. > > >> +static int qcom_flash_strobe_set(struct led_classdev_flash *fled_cdev, bool state) >> +{ >> + struct qcom_flash_led *led = flcdev_to_qcom_fled(fled_cdev); >> + int rc; >> + >> + rc = set_flash_current(led, led->flash_current_ma, FLASH_MODE); >> + if (rc) >> + return rc; >> + >> + rc = set_flash_timeout(led, led->flash_timeout_ms); >> + if (rc) >> + return rc; >> + >> + rc = set_flash_module_en(led, state); >> + if (rc) >> + return rc; >> + >> + return set_flash_strobe(led, SW_STROBE, state); >> +} > > Should we disable the module before setting the current? It might be > already active due to torch mode... > The module enabling status should be kept as it is because it may be still controlling other LED channels, I can strobe off the LED before setting the current to cover such case. I will update it as a following patch. > >> + return -EINVAL; >> + } >> + >> + flash_data->v4l2_flash = devm_kcalloc(dev, count, >> + sizeof(*flash_data->v4l2_flash), GFP_KERNEL); >> + if (!flash_data->v4l2_flash) >> + return -ENOMEM; >> + >> + device_for_each_child_node(dev, child) { >> + led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL); >> + if (!led) { >> + rc = -ENOMEM; >> + goto release; >> + } >> + >> + led->flash_data = flash_data; >> + rc = qcom_flash_register_led_device(dev, child, led); >> + if (rc < 0) >> + goto release; >> + >> + flash_data->leds_count++; >> + } > > Do you need to do of_node_put in error path I will update this as well. > > BR, > Pavel
On 3/26/2023 1:36 AM, Pavel Machek wrote: > On Fri 2023-03-03 17:50:22, Fenglin Wu wrote: >> Add initial driver to support flash LED module found in Qualcomm >> Technologies, Inc. PMICs. The flash module can have 3 or 4 channels >> and each channel can be controlled indepedently and support full scale >> current up to 1.5 A. It also supports connecting two channels together >> to supply one LED component with full scale current up to 2 A. In that >> case, the current will be split on each channel symmetrically and the >> channels will be enabled and disabled at the same time. >> >> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com> >> Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sm7225-fairphone-fp4 + pm6150l > >> + flash->led_cdev.brightness_set_blocking = qcom_flash_led_brightness_set; >> + >> + init_data.fwnode = node; >> + init_data.devicename = NULL; >> + init_data.default_label = NULL; >> + init_data.devname_mandatory = false; >> + >> + rc = devm_led_classdev_flash_register_ext(dev, flash, &init_data); >> + if (rc < 0) { >> + dev_err(dev, "Register flash LED classdev failed, rc=%d\n", rc); >> + return rc; >> + } > > I'd expect setting max_brightness somewhere around here. > > What does cat /sys/class/leds/*/max_brightness say (and what directory > name are you using)? LED_FULL is used as the default max_brightness, the driver will do the calculation based on LED_FULL (as below) and set the corresponding current when brightness is updated : u32 current_ma = brightness * led->max_torch_current_ma / LED_FULL; This is how max_brightness shows on my device: # cat /sys/class/leds/white:flash-0/max_brightness 255 > > BR, Pavel
Initial driver and binding document changes for supporting flash LED module in Qualcomm Technologies, Inc. PMICs. Changes in V8: 1. Introduce a helper function 'flcdev_to_qcom_fled()'. 2. Rename 'struct qcom_flash_chip' as 'struct qcom_flash_data' and remove 'qcom_flash_led' data pointers from it to avoid interwoven reference. Also remove the 'dev' pointer. 3. Remove 'v4l2_flash' data pointer in 'struct qcom_flash_led' and move it to 'struct qcom_flash_data'. Changes in V7: 1. Fix compilation issue reported by kernel test robot Changes in V6: 1. Update the driver to address review comments from Jones,Lee. Changes in V5: 1. Add MODULE_DEVICE_TABLE for auto-loading. Changes in V4: 1. Added Tested-By tag. 2. Addressed review comments in the binding change and added Reviewed-by tag. Changes in V3: 1. Updated the driver to use regmap_field for register access. 2. Adressed the review comments in binding document change. Changes in V2: 1. Addressed review comments in binding change, thanks Krzysztof! 2. Updated driver to address the compilation issue reported by kernel test robot. Fenglin Wu (2): leds: flash: add driver to support flash LED module in QCOM PMICs dt-bindings: leds: add QCOM flash LED controller .../bindings/leds/qcom,spmi-flash-led.yaml | 116 +++ drivers/leds/flash/Kconfig | 15 + drivers/leds/flash/Makefile | 1 + drivers/leds/flash/leds-qcom-flash.c | 773 ++++++++++++++++++ 4 files changed, 905 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml create mode 100644 drivers/leds/flash/leds-qcom-flash.c