Hi Jacek,

Thanks for the patchset.

Jacek Anaszewski wrote:

Which registers are these definitions related to? Could they be defined
next to the registers instead?

You could parameterise these macros, e.g.

#define MAX77693_FLASH_EN_SHIFT(a)	(6 - ((a) - 1) * 2)

+#define MAX77693_FLASH_LOW_BATTERY_EN	0x80
+
+#define MAX77693_FLASH_BOOST_FIXED	0x04
+#define MAX77693_FLASH_BOOST_LEDNUM_2	0x80
+
+#define MAX77693_FLASH_TIMEOUT_MIN	62500
+#define MAX77693_FLASH_TIMEOUT_MAX	1000000
+#define MAX77693_FLASH_TIMEOUT_STEP	62500
+
+#define MAX77693_TORCH_TIMEOUT_MIN	262000
+#define MAX77693_TORCH_TIMEOUT_MAX	15728000
+
+#define MAX77693_FLASH_IOUT_MIN		15625
+#define MAX77693_FLASH_IOUT_MAX_1LED	1000000
+#define MAX77693_FLASH_IOUT_MAX_2LEDS	625000
+#define MAX77693_FLASH_IOUT_STEP	15625
+
+#define MAX77693_TORCH_IOUT_MIN		15625
+#define MAX77693_TORCH_IOUT_MAX		250000
+#define MAX77693_TORCH_IOUT_STEP	15625
+
+#define MAX77693_FLASH_VSYS_MIN		2400
+#define MAX77693_FLASH_VSYS_MAX		3400
+#define MAX77693_FLASH_VSYS_STEP	33
+
+#define MAX77693_FLASH_VOUT_MIN		3300
+#define MAX77693_FLASH_VOUT_MAX		5500
+#define MAX77693_FLASH_VOUT_STEP	25
+#define MAX77693_FLASH_VOUT_RMIN	0x0c
+
+#define MAX77693_LED_STATUS_FLASH_ON	(1 << 3)
+#define MAX77693_LED_STATUS_TORCH_ON	(1 << 2)
+
+#define MAX77693_LED_FLASH_INT_FLED2_OPEN	(1 << 0)
+#define MAX77693_LED_FLASH_INT_FLED2_SHORT	(1 << 1)
+#define MAX77693_LED_FLASH_INT_FLED1_OPEN	(1 << 2)
+#define MAX77693_LED_FLASH_INT_FLED1_SHORT	(1 << 3)
+#define MAX77693_LED_FLASH_INT_OVER_CURRENT	(1 << 4)
+
+#define MAX77693_MODE_OFF			0
+#define MAX77693_MODE_FLASH1			(1 << 0)
+#define MAX77693_MODE_FLASH2			(1 << 1)
+#define MAX77693_MODE_TORCH1			(1 << 2)
+#define MAX77693_MODE_TORCH2			(1 << 3)
+#define MAX77693_MODE_FLASH_EXTERNAL1		(1 << 4)
+#define MAX77693_MODE_FLASH_EXTERNAL2		(1 << 5)
+
+enum {
+	FLED1,
+	FLED2
+};
+
+enum {
+	FLASH,
+	TORCH
+};
+
+struct max77693_led {
+	struct regmap *regmap;
+	struct platform_device *pdev;
+	struct max77693_led_platform_data *pdata;
+	struct mutex lock;
+
+	struct led_classdev_flash ldev1;
+	struct work_struct work1_brightness_set;
+	struct v4l2_flash *v4l2_flash1;
+
+	struct led_classdev_flash ldev2;
+	struct work_struct work2_brightness_set;
+	struct v4l2_flash *v4l2_flash2;
+
+	unsigned int torch1_brightness;
+	unsigned int torch2_brightness;
+	unsigned int flash1_timeout;
+	unsigned int flash2_timeout;

Could you use e.g. an array of structs instead? You could remove a lot
of redundant code operating on the other led.

+	unsigned int current_flash_timeout;
+	unsigned int mode_flags;
+	u8 torch_iout_reg;
+	bool iout_joint;
+};

...

+static int max77693_led_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct max77693_dev *iodev = dev_get_drvdata(dev->parent);
+	struct max77693_led *led;
+	struct max77693_led_platform_data *p;
+	int ret;
+
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pdev = pdev;
+	led->regmap = iodev->regmap;
+	platform_set_drvdata(pdev, led);
+	ret = max77693_led_get_platform_data(led);
+	if (ret < 0)
+		return -EINVAL;
+
+	p = led->pdata;
+	mutex_init(&led->lock);
+
+	if (p->num_leds == 1 && p->fleds[FLED1] && p->fleds[FLED2])
+		led->iout_joint = true;
+
+	ret = max77693_setup(led);
+	if (ret < 0)
+		return ret;

goto err_register_led1;

+	if (led->iout_joint || p->fleds[FLED1]) {
+		ret = max77693_register_led1(led);
+		if (ret < 0)
+			goto err_register_led1;
+	}
+
+	if (!led->iout_joint && p->fleds[FLED2]) {
+		ret = max77693_register_led2(led);
+		if (ret < 0)
+			goto err_register_led2;
+	}
+
+	return 0;
+
+err_register_led2:
+	if (!p->fleds[FLED1])
+		goto err_register_led1;
+	v4l2_flash_release(led->v4l2_flash1);
+	led_classdev_flash_unregister(&led->ldev1);
+err_register_led1:
+	mutex_destroy(&led->lock);
+
+	return ret;
+}

...

-- 
Kind regards,

Sakari Ailus
sakari.ailus@iki.fi