Re: [PATCH V4 4/8] backlight: qcom-wled: Rename PM8941* to WLED3

From: Bjorn Andersson
Date: Tue Aug 07 2018 - 01:38:28 EST

Next message: Christoph Hellwig: "Re: [PATCH v2 4/9] target: don't depend on SCSI"
Previous message: Bjorn Andersson: "Re: [PATCH V4 5/8] backlight: qcom-wled: Restructure the driver for WLED3"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon 09 Jul 03:22 PDT 2018, Kiran Gunda wrote:

> Rename the PM8941* references as WLED3 to make the
> driver generic and have WLED support for other PMICs.
>
> Signed-off-by: Kiran Gunda <kgunda@xxxxxxxxxxxxxx>

I agree with Daniel, regarding the mentioning of the variable rename.

Apart from that:

Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>

Regards,
Bjorn

> ---
> Changes from V3:
> - Changed the MODULE_DESCRIPTION
>
> 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..3cd6e75 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
> -#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
>
> -#define PM8941_WLED_REG_SINK 0x4f
> -#define PM8941_WLED_REG_SINK_MASK 0xe0
> -#define PM8941_WLED_REG_SINK_SHFT 0x05
> +#define WLED3_SINK_REG_CURR_SINK 0x4f
> +#define WLED3_SINK_REG_CURR_SINK_MASK 0xe0
> +#define WLED3_SINK_REG_CURR_SINK_SHFT 0x05
>
> -/* Per-'string' registers below */
> -#define PM8941_WLED_REG_STR_OFFSET 0x10
> +/* WLED3 per-'string' registers below */
> +#define WLED3_SINK_REG_STR_OFFSET 0x10
>
> -#define PM8941_WLED_REG_STR_MOD_EN_BASE 0x60
> -#define PM8941_WLED_REG_STR_MOD_MASK BIT(7)
> -#define PM8941_WLED_REG_STR_MOD_EN BIT(7)
> +#define WLED3_SINK_REG_STR_MOD_EN_BASE 0x60
> +#define WLED3_SINK_REG_STR_MOD_MASK BIT(7)
> +#define WLED3_SINK_REG_STR_MOD_EN BIT(7)
>
> -#define PM8941_WLED_REG_STR_SCALE_BASE 0x62
> -#define PM8941_WLED_REG_STR_SCALE_MASK 0x1f
> +#define WLED3_SINK_REG_STR_FULL_SCALE_CURR 0x62
> +#define WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK 0x1f
>
> -#define PM8941_WLED_REG_STR_MOD_SRC_BASE 0x63
> -#define PM8941_WLED_REG_STR_MOD_SRC_MASK 0x01
> -#define PM8941_WLED_REG_STR_MOD_SRC_INT 0x00
> -#define PM8941_WLED_REG_STR_MOD_SRC_EXT 0x01
> +#define WLED3_SINK_REG_STR_MOD_SRC_BASE 0x63
> +#define WLED3_SINK_REG_STR_MOD_SRC_MASK 0x01
> +#define WLED3_SINK_REG_STR_MOD_SRC_INT 0x00
> +#define WLED3_SINK_REG_STR_MOD_SRC_EXT 0x01
>
> -#define PM8941_WLED_REG_STR_CABC_BASE 0x66
> -#define PM8941_WLED_REG_STR_CABC_MASK BIT(7)
> -#define PM8941_WLED_REG_STR_CABC_EN BIT(7)
> +#define WLED3_SINK_REG_STR_CABC_BASE 0x66
> +#define WLED3_SINK_REG_STR_CABC_MASK BIT(7)
> +#define WLED3_SINK_REG_STR_CABC_EN BIT(7)
>
> -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;
> bool cs_out_en;
> bool ext_gen;
> bool cabc_en;
> };
>
> -struct pm8941_wled {
> +struct wled {
> const char *name;
> struct regmap *regmap;
> u16 addr;
>
> - struct pm8941_wled_config cfg;
> + struct wled_config cfg;
> };
>
> -static int pm8941_wled_update_status(struct backlight_device *bl)
> +static int wled_update_status(struct backlight_device *bl)
> {
> - struct pm8941_wled *wled = bl_get_data(bl);
> + struct wled *wled = bl_get_data(bl);
> u16 val = bl->props.brightness;
> u8 ctrl = 0;
> int rc;
> @@ -100,11 +102,11 @@ static int pm8941_wled_update_status(struct backlight_device *bl)
> val = 0;
>
> if (val != 0)
> - ctrl = PM8941_WLED_REG_MOD_EN_BIT;
> + ctrl = WLED3_CTRL_REG_MOD_EN_BIT;
>
> rc = regmap_update_bits(wled->regmap,
> - wled->addr + PM8941_WLED_REG_MOD_EN,
> - PM8941_WLED_REG_MOD_EN_MASK, ctrl);
> + wled->addr + WLED3_CTRL_REG_MOD_EN,
> + WLED3_CTRL_REG_MOD_EN_MASK, ctrl);
> if (rc)
> return rc;
>
> @@ -112,89 +114,89 @@ static int pm8941_wled_update_status(struct backlight_device *bl)
> u8 v[2] = { val & 0xff, (val >> 8) & 0xf };
>
> rc = regmap_bulk_write(wled->regmap,
> - wled->addr + PM8941_WLED_REG_VAL_BASE + 2 * i,
> + wled->addr + WLED3_CTRL_REG_VAL_BASE + 2 * i,
> v, 2);
> if (rc)
> return rc;
> }
>
> rc = regmap_update_bits(wled->regmap,
> - wled->addr + PM8941_WLED_REG_SYNC,
> - PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_ALL);
> + wled->addr + WLED3_SINK_REG_SYNC,
> + WLED3_SINK_REG_SYNC_MASK, WLED3_SINK_REG_SYNC_ALL);
> if (rc)
> return rc;
>
> rc = regmap_update_bits(wled->regmap,
> - wled->addr + PM8941_WLED_REG_SYNC,
> - PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_CLEAR);
> + wled->addr + WLED3_SINK_REG_SYNC,
> + WLED3_SINK_REG_SYNC_MASK, WLED3_SINK_REG_SYNC_CLEAR);
> return rc;
> }
>
> -static int pm8941_wled_setup(struct pm8941_wled *wled)
> +static int wled_setup(struct wled *wled)
> {
> int rc;
> int i;
>
> rc = regmap_update_bits(wled->regmap,
> - wled->addr + PM8941_WLED_REG_OVP,
> - PM8941_WLED_REG_OVP_MASK, wled->cfg.ovp);
> + wled->addr + WLED3_CTRL_REG_OVP,
> + WLED3_CTRL_REG_OVP_MASK, wled->cfg.ovp);
> if (rc)
> return rc;
>
> rc = regmap_update_bits(wled->regmap,
> - wled->addr + PM8941_WLED_REG_BOOST,
> - PM8941_WLED_REG_BOOST_MASK, wled->cfg.i_boost_limit);
> + wled->addr + WLED3_CTRL_REG_ILIMIT,
> + WLED3_CTRL_REG_ILIMIT_MASK, wled->cfg.boost_i_limit);
> if (rc)
> return rc;
>
> rc = regmap_update_bits(wled->regmap,
> - wled->addr + PM8941_WLED_REG_FREQ,
> - PM8941_WLED_REG_FREQ_MASK, wled->cfg.switch_freq);
> + wled->addr + WLED3_CTRL_REG_FREQ,
> + WLED3_CTRL_REG_FREQ_MASK, wled->cfg.switch_freq);
> if (rc)
> return rc;
>
> if (wled->cfg.cs_out_en) {
> u8 all = (BIT(wled->cfg.num_strings) - 1)
> - << PM8941_WLED_REG_SINK_SHFT;
> + << WLED3_SINK_REG_CURR_SINK_SHFT;
>
> rc = regmap_update_bits(wled->regmap,
> - wled->addr + PM8941_WLED_REG_SINK,
> - PM8941_WLED_REG_SINK_MASK, all);
> + wled->addr + WLED3_SINK_REG_CURR_SINK,
> + WLED3_SINK_REG_CURR_SINK_MASK, all);
> if (rc)
> return rc;
> }
>
> for (i = 0; i < wled->cfg.num_strings; ++i) {
> - u16 addr = wled->addr + PM8941_WLED_REG_STR_OFFSET * i;
> + u16 addr = wled->addr + WLED3_SINK_REG_STR_OFFSET * i;
>
> rc = regmap_update_bits(wled->regmap,
> - addr + PM8941_WLED_REG_STR_MOD_EN_BASE,
> - PM8941_WLED_REG_STR_MOD_MASK,
> - PM8941_WLED_REG_STR_MOD_EN);
> + addr + WLED3_SINK_REG_STR_MOD_EN_BASE,
> + WLED3_SINK_REG_STR_MOD_MASK,
> + WLED3_SINK_REG_STR_MOD_EN);
> if (rc)
> return rc;
>
> if (wled->cfg.ext_gen) {
> rc = regmap_update_bits(wled->regmap,
> - addr + PM8941_WLED_REG_STR_MOD_SRC_BASE,
> - PM8941_WLED_REG_STR_MOD_SRC_MASK,
> - PM8941_WLED_REG_STR_MOD_SRC_EXT);
> + addr + WLED3_SINK_REG_STR_MOD_SRC_BASE,
> + WLED3_SINK_REG_STR_MOD_SRC_MASK,
> + WLED3_SINK_REG_STR_MOD_SRC_EXT);
> if (rc)
> return rc;
> }
>
> rc = regmap_update_bits(wled->regmap,
> - addr + PM8941_WLED_REG_STR_SCALE_BASE,
> - PM8941_WLED_REG_STR_SCALE_MASK,
> - wled->cfg.i_limit);
> + addr + WLED3_SINK_REG_STR_FULL_SCALE_CURR,
> + WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK,
> + wled->cfg.string_i_limit);
> if (rc)
> return rc;
>
> rc = regmap_update_bits(wled->regmap,
> - addr + PM8941_WLED_REG_STR_CABC_BASE,
> - PM8941_WLED_REG_STR_CABC_MASK,
> + addr + WLED3_SINK_REG_STR_CABC_BASE,
> + WLED3_SINK_REG_STR_CABC_MASK,
> wled->cfg.cabc_en ?
> - PM8941_WLED_REG_STR_CABC_EN : 0);
> + WLED3_SINK_REG_STR_CABC_EN : 0);
> if (rc)
> return rc;
> }
> @@ -202,9 +204,9 @@ static int pm8941_wled_setup(struct pm8941_wled *wled)
> return 0;
> }
>
> -static const struct pm8941_wled_config pm8941_wled_config_defaults = {
> - .i_boost_limit = 3,
> - .i_limit = 20,
> +static const struct wled_config wled3_config_defaults = {
> + .boost_i_limit = 3,
> + .string_i_limit = 20,
> .ovp = 2,
> .switch_freq = 5,
> .num_strings = 0,
> @@ -213,55 +215,55 @@ static int pm8941_wled_setup(struct pm8941_wled *wled)
> .cabc_en = false,
> };
>
> -struct pm8941_wled_var_cfg {
> +struct wled_var_cfg {
> const u32 *values;
> u32 (*fn)(u32);
> int size;
> };
>
> -static const u32 pm8941_wled_i_boost_limit_values[] = {
> +static const u32 wled3_boost_i_limit_values[] = {
> 105, 385, 525, 805, 980, 1260, 1400, 1680,
> };
>
> -static const struct pm8941_wled_var_cfg pm8941_wled_i_boost_limit_cfg = {
> - .values = pm8941_wled_i_boost_limit_values,
> - .size = ARRAY_SIZE(pm8941_wled_i_boost_limit_values),
> +static const struct wled_var_cfg wled3_boost_i_limit_cfg = {
> + .values = wled3_boost_i_limit_values,
> + .size = ARRAY_SIZE(wled3_boost_i_limit_values),
> };
>
> -static const u32 pm8941_wled_ovp_values[] = {
> +static const u32 wled3_ovp_values[] = {
> 35, 32, 29, 27,
> };
>
> -static const struct pm8941_wled_var_cfg pm8941_wled_ovp_cfg = {
> - .values = pm8941_wled_ovp_values,
> - .size = ARRAY_SIZE(pm8941_wled_ovp_values),
> +static const struct wled_var_cfg wled3_ovp_cfg = {
> + .values = wled3_ovp_values,
> + .size = ARRAY_SIZE(wled3_ovp_values),
> };
>
> -static u32 pm8941_wled_num_strings_values_fn(u32 idx)
> +static u32 wled3_num_strings_values_fn(u32 idx)
> {
> return idx + 1;
> }
>
> -static const struct pm8941_wled_var_cfg pm8941_wled_num_strings_cfg = {
> - .fn = pm8941_wled_num_strings_values_fn,
> +static const struct wled_var_cfg wled3_num_strings_cfg = {
> + .fn = wled3_num_strings_values_fn,
> .size = 3,
> };
>
> -static u32 pm8941_wled_switch_freq_values_fn(u32 idx)
> +static u32 wled3_switch_freq_values_fn(u32 idx)
> {
> return 19200 / (2 * (1 + idx));
> }
>
> -static const struct pm8941_wled_var_cfg pm8941_wled_switch_freq_cfg = {
> - .fn = pm8941_wled_switch_freq_values_fn,
> +static const struct wled_var_cfg wled3_switch_freq_cfg = {
> + .fn = wled3_switch_freq_values_fn,
> .size = 16,
> };
>
> -static const struct pm8941_wled_var_cfg pm8941_wled_i_limit_cfg = {
> +static const struct wled_var_cfg wled3_string_i_limit_cfg = {
> .size = 26,
> };
>
> -static u32 pm8941_wled_values(const struct pm8941_wled_var_cfg *cfg, u32 idx)
> +static u32 wled3_values(const struct wled_var_cfg *cfg, u32 idx)
> {
> if (idx >= cfg->size)
> return UINT_MAX;
> @@ -272,9 +274,9 @@ static u32 pm8941_wled_values(const struct pm8941_wled_var_cfg *cfg, u32 idx)
> return idx;
> }
>
> -static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)
> +static int wled_configure(struct wled *wled, struct device *dev)
> {
> - struct pm8941_wled_config *cfg = &wled->cfg;
> + struct wled_config *cfg = &wled->cfg;
> u32 val;
> int rc;
> u32 c;
> @@ -284,32 +286,32 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)
> const struct {
> const char *name;
> u32 *val_ptr;
> - const struct pm8941_wled_var_cfg *cfg;
> + const struct wled_var_cfg *cfg;
> } u32_opts[] = {
> {
> "qcom,current-boost-limit",
> - &cfg->i_boost_limit,
> - .cfg = &pm8941_wled_i_boost_limit_cfg,
> + &cfg->boost_i_limit,
> + .cfg = &wled3_boost_i_limit_cfg,
> },
> {
> "qcom,current-limit",
> - &cfg->i_limit,
> - .cfg = &pm8941_wled_i_limit_cfg,
> + &cfg->string_i_limit,
> + .cfg = &wled3_string_i_limit_cfg,
> },
> {
> "qcom,ovp",
> &cfg->ovp,
> - .cfg = &pm8941_wled_ovp_cfg,
> + .cfg = &wled3_ovp_cfg,
> },
> {
> "qcom,switching-freq",
> &cfg->switch_freq,
> - .cfg = &pm8941_wled_switch_freq_cfg,
> + .cfg = &wled3_switch_freq_cfg,
> },
> {
> "qcom,num-strings",
> &cfg->num_strings,
> - .cfg = &pm8941_wled_num_strings_cfg,
> + .cfg = &wled3_num_strings_cfg,
> },
> };
> const struct {
> @@ -332,7 +334,7 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)
> if (rc)
> wled->name = dev->of_node->name;
>
> - *cfg = pm8941_wled_config_defaults;
> + *cfg = wled3_config_defaults;
> for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) {
> rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val);
> if (rc == -EINVAL) {
> @@ -344,7 +346,7 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)
>
> c = UINT_MAX;
> for (j = 0; c != val; j++) {
> - c = pm8941_wled_values(u32_opts[i].cfg, j);
> + c = wled3_values(u32_opts[i].cfg, j);
> if (c == UINT_MAX) {
> dev_err(dev, "invalid value for '%s'\n",
> u32_opts[i].name);
> @@ -366,15 +368,15 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)
> return 0;
> }
>
> -static const struct backlight_ops pm8941_wled_ops = {
> - .update_status = pm8941_wled_update_status,
> +static const struct backlight_ops wled_ops = {
> + .update_status = wled_update_status,
> };
>
> -static int pm8941_wled_probe(struct platform_device *pdev)
> +static int wled_probe(struct platform_device *pdev)
> {
> struct backlight_properties props;
> struct backlight_device *bl;
> - struct pm8941_wled *wled;
> + struct wled *wled;
> struct regmap *regmap;
> u32 val;
> int rc;
> @@ -391,42 +393,42 @@ static int pm8941_wled_probe(struct platform_device *pdev)
>
> wled->regmap = regmap;
>
> - rc = pm8941_wled_configure(wled, &pdev->dev);
> + rc = wled_configure(wled, &pdev->dev);
> if (rc)
> return rc;
>
> - rc = pm8941_wled_setup(wled);
> + rc = wled_setup(wled);
> if (rc)
> return rc;
>
> - val = PM8941_WLED_DEFAULT_BRIGHTNESS;
> + val = WLED_DEFAULT_BRIGHTNESS;
> of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
>
> memset(&props, 0, sizeof(struct backlight_properties));
> props.type = BACKLIGHT_RAW;
> props.brightness = val;
> - props.max_brightness = PM8941_WLED_REG_VAL_MAX;
> + props.max_brightness = WLED3_SINK_REG_BRIGHT_MAX;
> bl = devm_backlight_device_register(&pdev->dev, wled->name,
> &pdev->dev, wled,
> - &pm8941_wled_ops, &props);
> + &wled_ops, &props);
> return PTR_ERR_OR_ZERO(bl);
> };
>
> -static const struct of_device_id pm8941_wled_match_table[] = {
> +static const struct of_device_id wled_match_table[] = {
> { .compatible = "qcom,pm8941-wled" },
> {}
> };
> -MODULE_DEVICE_TABLE(of, pm8941_wled_match_table);
> +MODULE_DEVICE_TABLE(of, wled_match_table);
>
> -static struct platform_driver pm8941_wled_driver = {
> - .probe = pm8941_wled_probe,
> +static struct platform_driver wled_driver = {
> + .probe = wled_probe,
> .driver = {
> - .name = "pm8941-wled",
> - .of_match_table = pm8941_wled_match_table,
> + .name = "qcom,wled",
> + .of_match_table = wled_match_table,
> },
> };
>
> -module_platform_driver(pm8941_wled_driver);
> +module_platform_driver(wled_driver);
>
> -MODULE_DESCRIPTION("pm8941 wled driver");
> +MODULE_DESCRIPTION("Qualcomm WLED driver");
> MODULE_LICENSE("GPL v2");
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

Next message: Christoph Hellwig: "Re: [PATCH v2 4/9] target: don't depend on SCSI"
Previous message: Bjorn Andersson: "Re: [PATCH V4 5/8] backlight: qcom-wled: Restructure the driver for WLED3"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]