On 2018-06-20 04:41, Bjorn Andersson wrote:
On Tue 19 Jun 04:13 PDT 2018, Kiran Gunda wrote:
Rename the PM8941* references as WLED3 to make the driver generic and have WLED support for other PMICs.
Looks good, just three minor comments below.
Signed-off-by: Kiran Gunda kgunda@codeaurora.org
drivers/video/backlight/qcom-wled.c | 248 ++++++++++++++++++------------------ 1 file changed, 125 insertions(+), 123 deletions(-)
diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 0b6d219..812f3cb 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -18,77 +18,79 @@ #include <linux/regmap.h>
/* From DT binding */ -#define PM8941_WLED_DEFAULT_BRIGHTNESS 2048 +#define WLED_DEFAULT_BRIGHTNESS 2048
-#define PM8941_WLED_REG_VAL_BASE 0x40 -#define PM8941_WLED_REG_VAL_MAX 0xFFF +#define WLED3_SINK_REG_BRIGHT_MAX 0xFFF +#define WLED3_CTRL_REG_VAL_BASE 0x40
-#define PM8941_WLED_REG_MOD_EN 0x46 -#define PM8941_WLED_REG_MOD_EN_BIT BIT(7) -#define PM8941_WLED_REG_MOD_EN_MASK BIT(7) +/* WLED3 control registers */ +#define WLED3_CTRL_REG_MOD_EN 0x46 +#define WLED3_CTRL_REG_MOD_EN_BIT BIT(7) +#define WLED3_CTRL_REG_MOD_EN_MASK BIT(7)
-#define PM8941_WLED_REG_SYNC 0x47
These are in address order, any reason why WLED3_SINK_REG_SYNC moved down?
Actually, this is a sink specific register. That's why moved it to under the /* WLED3 sink specific registers */
-#define PM8941_WLED_REG_SYNC_MASK 0x07 -#define PM8941_WLED_REG_SYNC_LED1 BIT(0) -#define PM8941_WLED_REG_SYNC_LED2 BIT(1) -#define PM8941_WLED_REG_SYNC_LED3 BIT(2) -#define PM8941_WLED_REG_SYNC_ALL 0x07 -#define PM8941_WLED_REG_SYNC_CLEAR 0x00 +#define WLED3_CTRL_REG_FREQ 0x4c +#define WLED3_CTRL_REG_FREQ_MASK 0x0f
-#define PM8941_WLED_REG_FREQ 0x4c -#define PM8941_WLED_REG_FREQ_MASK 0x0f +#define WLED3_CTRL_REG_OVP 0x4d +#define WLED3_CTRL_REG_OVP_MASK 0x03
-#define PM8941_WLED_REG_OVP 0x4d -#define PM8941_WLED_REG_OVP_MASK 0x03 +#define WLED3_CTRL_REG_ILIMIT 0x4e +#define WLED3_CTRL_REG_ILIMIT_MASK 0x07
-#define PM8941_WLED_REG_BOOST 0x4e -#define PM8941_WLED_REG_BOOST_MASK 0x07 +/* WLED3 sink registers */ +#define WLED3_SINK_REG_SYNC 0x47 +#define WLED3_SINK_REG_SYNC_MASK 0x07 +#define WLED3_SINK_REG_SYNC_LED1 BIT(0) +#define WLED3_SINK_REG_SYNC_LED2 BIT(1) +#define WLED3_SINK_REG_SYNC_LED3 BIT(2) +#define WLED3_SINK_REG_SYNC_ALL 0x07 +#define WLED3_SINK_REG_SYNC_CLEAR 0x00
[..]
-struct pm8941_wled_config {
- u32 i_boost_limit;
+struct wled_config {
- u32 boost_i_limit; u32 ovp; u32 switch_freq; u32 num_strings;
- u32 i_limit;
- u32 string_i_limit;
Changing the members in this struct seems unrelated.
Changed the members, to have resemblance with the IPCAT and better readability .
bool cs_out_en; bool ext_gen; bool cabc_en; };
[..]
-MODULE_DESCRIPTION("pm8941 wled driver"); +MODULE_DESCRIPTION("qcom wled driver");
I would prefer if this was "Qualcomm WLED driver".
Sure. I will change it in the next series.
MODULE_LICENSE("GPL v2");
Regards, Bjorn