| Message ID | d1d1419bb322e2b0f40d34edd3a66979015b668c.1712657386.git.chiaen_wu@richtek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | leds: mt6360: Fix the second LED can not enable torch mode by V4L2 | expand |
Il 09/04/24 12:21, ChiaEn Wu ha scritto: > V4L2 will disable strobe mode of the LED device when enable torch mode, > but this logic will conflict with the "priv->fled_torch_used" > in "mt6360_strobe_set()". So after enabling torch mode of the first > LED, the second LED will not be able to enable torch mode correctly. > > Therefore, at the beginning of "mt6360_strobe_set()", check whether the > state of the upcoming change and the current LED device state are the > same, so as to avoid the above problem. > > Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com> > --- > drivers/leds/flash/leds-mt6360.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/leds/flash/leds-mt6360.c b/drivers/leds/flash/leds-mt6360.c > index a90de82f4568..1b75b4d36834 100644 > --- a/drivers/leds/flash/leds-mt6360.c > +++ b/drivers/leds/flash/leds-mt6360.c > @@ -241,10 +241,20 @@ static int mt6360_strobe_set(struct led_classdev_flash *fl_cdev, bool state) > u32 enable_mask = MT6360_STROBEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > u32 val = state ? MT6360_FLCSEN_MASK(led->led_no) : 0; > u32 prev = priv->fled_strobe_used, curr; > - int ret; > + int ret = 0; I prefer that you leave ret uninitialized here, and... > > mutex_lock(&priv->lock); > > + /* > + * If the state of the upcoming change is the same as the current LED > + * device state, then skip the subsequent code to avoid conflict > + * with the flow of turning on LED torch mode in V4L2. > + */ > + if (state == !!(BIT(led->led_no) & prev)) { > + dev_info(lcdev->dev, "No change in strobe state [0x%x]\n", prev); ...that you do here, instead ret = 0; goto unlock; With that addressed, Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collaobra.com>
On Tue, 09 Apr 2024, AngeloGioacchino Del Regno wrote: Not sure why, but this mail was sent unthreaded from the original. > Il 09/04/24 12:21, ChiaEn Wu ha scritto: > > V4L2 will disable strobe mode of the LED device when enable torch mode, > > but this logic will conflict with the "priv->fled_torch_used" > > in "mt6360_strobe_set()". So after enabling torch mode of the first > > LED, the second LED will not be able to enable torch mode correctly. > > > > Therefore, at the beginning of "mt6360_strobe_set()", check whether the > > state of the upcoming change and the current LED device state are the > > same, so as to avoid the above problem. > > > > Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com> > > --- > > drivers/leds/flash/leds-mt6360.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/leds/flash/leds-mt6360.c b/drivers/leds/flash/leds-mt6360.c > > index a90de82f4568..1b75b4d36834 100644 > > --- a/drivers/leds/flash/leds-mt6360.c > > +++ b/drivers/leds/flash/leds-mt6360.c > > @@ -241,10 +241,20 @@ static int mt6360_strobe_set(struct led_classdev_flash *fl_cdev, bool state) > > u32 enable_mask = MT6360_STROBEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > > u32 val = state ? MT6360_FLCSEN_MASK(led->led_no) : 0; > > u32 prev = priv->fled_strobe_used, curr; > > - int ret; > > + int ret = 0; > > I prefer that you leave ret uninitialized here, and... What's the reason for this? > > mutex_lock(&priv->lock); > > + /* > > + * If the state of the upcoming change is the same as the current LED > > + * device state, then skip the subsequent code to avoid conflict > > + * with the flow of turning on LED torch mode in V4L2. > > + */ > > + if (state == !!(BIT(led->led_no) & prev)) { > > + dev_info(lcdev->dev, "No change in strobe state [0x%x]\n", prev); > > ...that you do here, instead > > ret = 0; > goto unlock; > > With that addressed, > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collaobra.com> > >
diff --git a/drivers/leds/flash/leds-mt6360.c b/drivers/leds/flash/leds-mt6360.c index a90de82f4568..1b75b4d36834 100644 --- a/drivers/leds/flash/leds-mt6360.c +++ b/drivers/leds/flash/leds-mt6360.c @@ -241,10 +241,20 @@ static int mt6360_strobe_set(struct led_classdev_flash *fl_cdev, bool state) u32 enable_mask = MT6360_STROBEN_MASK | MT6360_FLCSEN_MASK(led->led_no); u32 val = state ? MT6360_FLCSEN_MASK(led->led_no) : 0; u32 prev = priv->fled_strobe_used, curr; - int ret; + int ret = 0; mutex_lock(&priv->lock); + /* + * If the state of the upcoming change is the same as the current LED + * device state, then skip the subsequent code to avoid conflict + * with the flow of turning on LED torch mode in V4L2. + */ + if (state == !!(BIT(led->led_no) & prev)) { + dev_info(lcdev->dev, "No change in strobe state [0x%x]\n", prev); + goto unlock; + } + /* * Only one set of flash control logic, use the flag to avoid torch is * currently used
V4L2 will disable strobe mode of the LED device when enable torch mode, but this logic will conflict with the "priv->fled_torch_used" in "mt6360_strobe_set()". So after enabling torch mode of the first LED, the second LED will not be able to enable torch mode correctly. Therefore, at the beginning of "mt6360_strobe_set()", check whether the state of the upcoming change and the current LED device state are the same, so as to avoid the above problem. Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com> --- drivers/leds/flash/leds-mt6360.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)