On 2/19/21 12:02 PM, Pavel Machek wrote:

Hi!

quoted

quoted

+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spmi.h>
+#include <linux/of_device.h>
+#include <linux/device.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/led-class-flash.h>
+#include <linux/regulator/consumer.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+#include <dt-bindings/leds/leds-qcom-spmi-flash.h>

Please sort includes alphabetically.

No need to do that.

Keeping the includes sorted eliminates the risk of introducing 
duplicates and allows for faster lookup.

What gain is in having them unsorted?

quoted

quoted

+#define FLASH_SAFETY_TIMER		0x40

Namespacing prefix is needed for macros, e.g. QCOM_FLASH*.

No need for that in .c files.

In general it eliminates the risk of name clash with other subsystems
headers.

And actually the prefix here should be QCOM_LED_FLASH to avoid ambiguity
with flash memory. If you dropped the vendor prefix then you'd get
possible name clash with led-class-flash.h namespace prefix.

-- 
Best regards,
Jacek Anaszewski