On Fri, Jul 22, 2022 at 12:25 PM ChiaEn Wu <peterwu.pub@xxxxxxxxx> wrote:
>
> From: Alice Chen <alice_chen@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
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
