Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 25 Jul 2022 10:51:06 +0200 | Subject | Re: [PATCH v6 12/13] leds: flash: mt6370: Add MediaTek MT6370 flashlight support |
| |
On Fri, Jul 22, 2022 at 12:25 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > > From: Alice Chen <alice_chen@richtek.com> > > The MediaTek MT6370 is a highly-integrated smart power management IC, > which includes a single cell Li-Ion/Li-Polymer switching battery > charger, a USB Type-C & Power Delivery (PD) controller, dual Flash > LED current sources, a RGB LED driver, a backlight WLED driver, > a display bias driver and a general LDO for portable devices. > > The Flash LED in MT6370 has 2 channels and support torch/strobe mode. > Add the support of MT6370 FLASH LED. > > Signed-off-by: Alice Chen <alice_chen@richtek.com>
This SoB chain is wrong. Prioritize and read Submitting Patches!
...
> +static int mt6370_torch_brightness_set(struct led_classdev *lcdev, > + enum led_brightness level) > +{ > + struct mt6370_led *led = to_mt6370_led(lcdev, flash.led_cdev); > + struct mt6370_priv *priv = led->priv; > + u32 led_enable_mask = (led->led_no == MT6370_LED_JOINT) ? > + MT6370_FLCSEN_MASK_ALL : > + MT6370_FLCSEN_MASK(led->led_no); > + u32 enable_mask = MT6370_TORCHEN_MASK | led_enable_mask; > + u32 val = level ? led_enable_mask : 0;
> + u32 prev = priv->fled_torch_used, curr;
Here and in the other functions with similar variables it seems you never use prev after assigning curr. Just define a single variable and use it accordingly.
> + int ret, i; > + > + mutex_lock(&priv->lock); > + > + /* > + * Only one set of flash control logic, > + * use the flag to avoid strobe is currently used. > + */ > + if (priv->fled_strobe_used) { > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", > + priv->fled_strobe_used); > + ret = -EBUSY; > + goto unlock; > + } > + > + if (level) > + curr = prev | BIT(led->led_no); > + else > + curr = prev & ~BIT(led->led_no); > + > + if (curr) > + val |= MT6370_TORCHEN_MASK; > + > + if (level) { > + level -= 1; > + if (led->led_no == MT6370_LED_JOINT) { > + int flevel[MT6370_MAX_LEDS]; > + > + flevel[0] = level / 2; > + flevel[1] = level - flevel[0]; > + for (i = 0; i < MT6370_MAX_LEDS; i++) { > + ret = regmap_update_bits(priv->regmap, > + MT6370_REG_FLEDITOR(i), > + MT6370_ITORCH_MASK, flevel[i]); > + if (ret) > + goto unlock; > + } > + } else { > + ret = regmap_update_bits(priv->regmap, > + MT6370_REG_FLEDITOR(led->led_no), > + MT6370_ITORCH_MASK, level); > + if (ret) > + goto unlock; > + } > + } > + > + ret = regmap_update_bits(priv->regmap, MT6370_REG_FLEDEN, > + enable_mask, val); > + if (ret) > + goto unlock; > + > + priv->fled_torch_used = curr; > + > +unlock: > + mutex_unlock(&priv->lock); > + return ret; > +}
...
> + struct v4l2_flash_config v4l2_config = {0};
0 is not needed.
-- With Best Regards, Andy Shevchenko
| |